All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] Gemalto: Improve init process
@ 2017-04-11  9:04 Vincent Cesson
  2017-04-11  9:04 ` [PATCH 1/1] gemalto: " Vincent Cesson
  0 siblings, 1 reply; 15+ messages in thread
From: Vincent Cesson @ 2017-04-11  9:04 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

This patch improves the robustness of Gemalto init process.
Interfaces don't show up sometimes, such as SimManager. In particular,
setting the modem Online->Offline->Online causes issue.
Also gprs reactivation is broken.

 - Change the Airplane mode to keep the SIM connected. It solves
run issue when setting Online again.
 - Use URC to enable SIM detection. It solves run issue on modem poweron.
 - Change the way/conditions to create the interfaces.
 - Shutdown the modem on exit.
 - Fix enable/disable return value.

Vincent Cesson (1):
  gemalto: Improve init process

 plugins/gemalto.c | 467 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 431 insertions(+), 36 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/1] gemalto: Improve init process
  2017-04-11  9:04 [PATCH 0/1] Gemalto: Improve init process Vincent Cesson
@ 2017-04-11  9:04 ` Vincent Cesson
  2017-04-11 12:30   ` [PATCH v2 " Vincent Cesson
  0 siblings, 1 reply; 15+ messages in thread
From: Vincent Cesson @ 2017-04-11  9:04 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 16754 bytes --]

Change Airplane mode: keep SIM connected (CFUN=4).
Fix enable/disable return value.
Create interfaces only if SIM card is detected.
Listen to URC for SIM status (improve SIM detection robustness).
Add some init commands.
---
 plugins/gemalto.c | 467 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 431 insertions(+), 36 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index ffe6814..03caaa8 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -42,6 +42,7 @@
 #include <ofono/message-waiting.h>
 #include <ofono/netreg.h>
 #include <ofono/phonebook.h>
+#include <ofono/cbs.h>
 #include <ofono/sim.h>
 #include <ofono/sms.h>
 #include <ofono/ussd.h>
@@ -53,10 +54,27 @@
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
+static void at_cpin_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data);
+
+static const char *sind_prefix[] = { "^SIND:", NULL };
+static const char *creg_prefix[] = { "+CREG:", NULL };
+static const char *cpin_prefix[] = { "+CPIN:", NULL };
+static const char *cfun_prefix[] = { "+CFUN:", NULL };
+static const char *sdport_prefix[] = { "^SDPORT:", NULL };
+static const char *gcap_prefix[] = { "+GCAP:", NULL };
+
+static gboolean sim_iface_created = FALSE;
 
 struct gemalto_data {
 	GAtChat *app;
 	GAtChat *mdm;
+	struct ofono_sim *sim;
+	gboolean pin_required;
+	gboolean need_reboot;
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
+	gboolean have_gsm;
 };
 
 static int gemalto_probe(struct ofono_modem *modem)
@@ -67,6 +85,10 @@ static int gemalto_probe(struct ofono_modem *modem)
 	if (data == NULL)
 		return -ENOMEM;
 
+	sim_iface_created = FALSE;
+	data->sim = NULL;
+	data->have_gsm = FALSE;
+
 	ofono_modem_set_data(modem, data);
 
 	return 0;
@@ -110,6 +132,125 @@ static GAtChat *open_device(const char *device)
 	return chat;
 }
 
+static void gemalto_cfun_enable_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+
+	DBG("");
+
+	/* Power on the modem */
+	ofono_modem_set_powered(modem, ok);
+}
+
+gboolean gemalto_modem_set_to_online(struct ofono_modem *modem)
+{
+	DBG("");
+	ofono_modem_set_powered(modem, TRUE);
+
+	/* Must return FALSE to stop the g_timeout loop */
+	return FALSE;
+}
+
+static void gemalto_cfun_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	gint init_online_state = 0;
+
+	DBG("");
+
+	if (!ok) {
+		/* In case of errors, the mode is not powered */
+		ofono_modem_set_powered(modem, FALSE);
+	} else {
+		g_at_result_iter_init(&iter, result);
+		g_at_result_iter_next(&iter, "+CFUN:");
+		g_at_result_iter_next_number(&iter, &init_online_state);
+
+		DBG("online = %d", init_online_state);
+		switch (init_online_state) {
+		case 0:
+			/* Put the modem offline */
+			g_at_chat_send(data->app, "AT+CFUN=4", NULL,
+					gemalto_cfun_enable_cb, modem, NULL);
+			break;
+		case 1:
+			/* Set modem to online and powered */
+			/* Wait for 300msec otherwise the sim manager has not finished
+			   and it is used in the NetworkManager.
+			   Otherwise, get a seg fault */
+			g_timeout_add(300, (GSourceFunc)gemalto_modem_set_to_online,
+					modem);
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+static void gemalto_sdport_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	gint sdport_mode = -1;
+
+	DBG("");
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+	g_at_result_iter_next(&iter, "^SDPORT:");
+	g_at_result_iter_next_number(&iter, &sdport_mode);
+
+	/* Already correct mode. */
+	if (sdport_mode == 3) {
+		/* Check the modem mode (online/offline) */
+		g_at_chat_send(data->app, "AT+CFUN?", cfun_prefix,
+				gemalto_cfun_query_cb, modem, NULL);
+	}
+}
+
+static void gemalto_smso_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	/* Try hard shutdown */
+	if (!ok) {
+		DBG("Error");
+		return;
+	}
+
+	g_at_chat_cancel_all(data->app);
+	g_at_chat_unregister_all(data->app);
+
+	g_at_chat_unref(data->mdm);
+	data->mdm = NULL;
+	g_at_chat_unref(data->app);
+	data->app = NULL;
+
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+void gemalto_shutdown(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	/* Shutdown the modem */
+	g_at_chat_send(data->app, "AT^SMSO", NULL, gemalto_smso_cb,
+			modem, NULL);
+}
+
 static int gemalto_enable(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
@@ -140,7 +281,24 @@ static int gemalto_enable(struct ofono_modem *modem)
 		g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
 	}
 
-	return 0;
+	g_at_chat_send(data->app, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
+
+	g_at_chat_send(data->app, "AT^SQPORT?", NULL, NULL, NULL, NULL);
+
+	/* TEMPORARY : Add this command to get data reconnection working */
+	g_at_chat_send(data->mdm, "AT&C0", NULL, NULL, NULL, NULL);
+
+	/* Configure the modem to be in profil 1 */
+	g_at_chat_send(data->app, "AT^SCFG=\"Radio/Mtpl\",1,1",
+			NULL, NULL, NULL, NULL);
+
+	g_at_chat_send(data->app, "AT^SDPORT?", sdport_prefix,
+			gemalto_sdport_query_cb, modem, NULL);
+
+	/* Add the Time Zone URC */
+	g_at_chat_send(data->app, "AT+CTZU=1", NULL, NULL, NULL, NULL);
+
+	return -EINPROGRESS;
 }
 
 static int gemalto_disable(struct ofono_modem *modem)
@@ -149,55 +307,272 @@ static int gemalto_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	g_at_chat_send(data->app, "AT^SMSO", NULL, NULL, NULL, NULL);
+	if (data->app == NULL)
+		return 0;
 
-	ofono_modem_set_data(modem, NULL);
+	gemalto_shutdown(modem);
 
-	return 0;
+	return -EINPROGRESS;
 }
 
-static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
+static void gemalto_cfun_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
 	ofono_modem_online_cb_t cb = cbd->cb;
-	struct ofono_error error;
-
-	decode_at_error(&error, g_at_result_final_response(result));
 
-	cb(&error, cbd->data);
+	if (ok)
+		CALLBACK_WITH_SUCCESS(cb, cbd->data);
+	else
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
 }
 
-static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
+static void gemalto_set_online(struct ofono_modem *modem, gboolean online,
 		ofono_modem_online_cb_t cb, void *user_data)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
 	struct cb_data *cbd = cb_data_new(cb, user_data);
-	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=0";
+	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=4";
 
 	DBG("modem %p %s", modem, online ? "online" : "offline");
 
-	if (g_at_chat_send(data->app, command, NULL, set_online_cb, cbd, g_free))
+	if (data->app == NULL) {
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		g_free(cbd);
+	}
+
+	g_at_chat_send(data->app, command, cfun_prefix, gemalto_cfun_cb, cbd,
+			g_free);
+}
+
+static void gemalto_create_sim(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	if (!sim_iface_created) {
+		sim_iface_created = TRUE;
+		/* Create the sim */
+		data->sim = ofono_sim_create(modem, 0, "atmodem",
+				data->app);
+		if (data->sim)
+			ofono_sim_inserted_notify(data->sim, TRUE);
+	}
+}
+
+static void gemalto_set_network(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	ofono_devinfo_create(modem, 0, "atmodem", data->app);
+
+	if (data->have_gsm == TRUE) {
+		ofono_sms_create(modem, 0, "atmodem", data->app);
+
+		ofono_cbs_create(modem, 0, "atmodem", data->app);
+
+		data->gprs = ofono_gprs_create(modem, 0,
+				"atmodem", data->app);
+
+		data->gc = ofono_gprs_context_create(modem, 0,
+				"atmodem", data->mdm);
+
+		if (data->gprs && data->gc)
+			ofono_gprs_add_context(data->gprs, data->gc);
+	}
+}
+
+static void gemalto_sind_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int value = 0;
+
+	DBG("");
+
+	if (ok) {
+		g_at_result_iter_init(&iter, result);
+		if (!g_at_result_iter_next(&iter, "^SIND:"))
+			return;
+		if (!g_at_result_iter_skip_next(&iter))
+			return;
+		if (!g_at_result_iter_skip_next(&iter))
+			return;
+		if (!g_at_result_iter_next_number(&iter, &value))
+			return;
+
+		if (value == 5) { /* SIM data ready */
+			/* If no pin required, we create the SIM interface */
+			if (!data->pin_required) {
+				/* Deactivate the SIND URC*/
+				g_at_chat_send(data->app, "AT^SIND=\"simstatus\",0",
+						sind_prefix, NULL, NULL, NULL);
+				gemalto_create_sim(modem);
+			}
+		}
+
+		g_at_result_iter_close_list(&iter);
+	}
+}
+
+static void gemalto_sind_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	char *signal_identifier = "simstatus";
+	const char *str;
+	int value = 0;
+
+	DBG("");
+
+	g_at_result_iter_init(&iter, result);
+	if (!g_at_result_iter_next(&iter, "+CIEV:"))
+		return;
+	if (!g_at_result_iter_next_unquoted_string(&iter, &str))
+		return;
+	if (g_str_equal(signal_identifier, str) == FALSE)
 		return;
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+	if (value == 5) { /* SIM data ready */
+		/* If no pin required, we create the SIM interface */
+		if (!data->pin_required)
+			gemalto_create_sim(modem);
+	}
+	g_at_result_iter_close_list(&iter);
+}
+
+static void gemalto_creg_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int value = 0;
+
+	DBG("");
+
+	if (ok) {
+		g_at_result_iter_init(&iter, result);
+		if (!g_at_result_iter_next(&iter, "+CREG:"))
+			return;
+		/* Skip the URC mode */
+		if (!g_at_result_iter_skip_next(&iter))
+			return;
+		if (!g_at_result_iter_next_number(&iter, &value))
+			return;
+		if (value == 5) {
+			/* Deactivate the CREG URC */
+			g_at_chat_send(data->app, "AT+CREG=0", NULL, NULL, NULL, NULL);
+
+			gemalto_set_network(modem);
+		}
+		g_at_result_iter_close_list(&iter);
+	}
+}
 
-	CALLBACK_WITH_FAILURE(cb, cbd->data);
+static void gemalto_creg_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int value = 0;
+
+	DBG("");
 
-	g_free(cbd);
+	g_at_result_iter_init(&iter, result);
+	if (!g_at_result_iter_next(&iter, "+CREG:"))
+		return;
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+
+	if (value == 5 && !data->have_gsm) {
+		data->have_gsm = TRUE;
+		gemalto_set_network(modem);
+	}
+	g_at_result_iter_close_list(&iter);
+}
+
+static gboolean at_pin_query(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	/* Invert FALSE = stop, TRUE = re-do to be used in g_timeout_add_seconds */
+	gboolean ret = FALSE;
+
+	if (g_at_chat_send(data->mdm, "AT+CPIN?", cpin_prefix,
+				at_cpin_query_cb, modem, NULL) <= 0) {
+		DBG("Error while querying the PIN.");
+		ret = TRUE;
+	}
+	return ret;
+}
+
+static void at_cpin_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	struct ofono_error error;
+	const char *pin_type_str;
+
+	if (ok) {
+		g_at_result_iter_init(&iter, result);
+
+		if (g_at_result_iter_next(&iter, "+CPIN:")) {
+			g_at_result_iter_next_unquoted_string(&iter, &pin_type_str);
+			/* No PIN required or there is an error */
+			if (g_strcmp0(pin_type_str, "READY") == 0) {
+				data->pin_required = FALSE;
+			} else { /* PIN is required */
+				data->pin_required = TRUE;
+				/* Create the SIM manager to enter PIN */
+				gemalto_create_sim(modem);
+			}
+		}
+	} else {
+		decode_at_error(&error, g_at_result_final_response(result));
+		/* If the error is CME busy */
+		if (error.type == OFONO_ERROR_TYPE_CME && error.error == 14) {
+			DBG("Invalid password => retry the command");
+			/* We re-try the command */
+			g_timeout_add_seconds(1, (GSourceFunc)at_pin_query, modem);
+			/* If the SIM is not inserted */
+		} else if (error.type == OFONO_ERROR_TYPE_CME && error.error == 11) {
+
+			DBG("PIN not entered, please enter PIN");
+			g_timeout_add_seconds(1, (GSourceFunc)at_pin_query, modem);
+
+		}
+	}
 }
 
 static void gemalto_pre_sim(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
-	struct ofono_sim *sim;
 
 	DBG("%p", modem);
 
 	ofono_devinfo_create(modem, 0, "atmodem", data->app);
-	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
-	ofono_voicecall_create(modem, 0, "atmodem", data->app);
 	ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
 
-	if (sim)
-		ofono_sim_inserted_notify(sim, TRUE);
+	/* Listen to simstatus SIND URC */
+	g_at_chat_register(data->app, "+CIEV:",
+			gemalto_sind_notify, FALSE, modem, NULL);
+	/* Listen to CREG URC */
+	g_at_chat_register(data->app, "+CREG:",
+			gemalto_creg_notify, FALSE, modem, NULL);
+
+	/* Check the SIM status and enable simstatus SIND URC */
+	g_at_chat_send(data->mdm, "AT^SIND=\"simstatus\",1", sind_prefix,
+			gemalto_sind_cb, modem, NULL);
+
+	/* Query the PIN command to create SIM Manager (or not) */
+	at_pin_query(modem);
 }
 
 static void gemalto_post_sim(struct ofono_modem *modem)
@@ -207,35 +582,55 @@ static void gemalto_post_sim(struct ofono_modem *modem)
 	DBG("%p", modem);
 
 	ofono_phonebook_create(modem, 0, "atmodem", data->app);
+}
+
+static void gcap_support(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	const char *gcap;
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+GCAP:"))
+		return;
 
-	ofono_sms_create(modem, 0, "atmodem", data->app);
+	while (g_at_result_iter_next_unquoted_string(&iter, &gcap)) {
+		if (*gcap == '\0')
+			break;
+
+		if (!g_strcmp0(gcap, "+CGSM"))
+			data->have_gsm = TRUE;
+	}
+
+	if (data->have_gsm)
+		gemalto_set_network(modem);
 }
 
 static void gemalto_post_online(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
-	struct ofono_message_waiting *mw;
-	struct ofono_gprs *gprs;
-	struct ofono_gprs_context *gc;
 
 	DBG("%p", modem);
 
-	ofono_ussd_create(modem, 0, "atmodem", data->app);
-	ofono_call_forwarding_create(modem, 0, "atmodem", data->app);
-	ofono_call_settings_create(modem, 0, "atmodem", data->app);
-	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
-	ofono_call_meter_create(modem, 0, "atmodem", data->app);
-	ofono_call_barring_create(modem, 0, "atmodem", data->app);
+	/* Enable CREG URC and check CREG status */
+	g_at_chat_send(data->app, "AT+CREG=1", creg_prefix, NULL, modem, NULL);
+	g_at_chat_send(data->app, "AT+CREG?", creg_prefix,
+			gemalto_creg_query_cb, modem, NULL);
 
-	gprs = ofono_gprs_create(modem, 0, "atmodem", data->app);
-	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
+	ofono_voicecall_create(modem, 0, "atmodem", data->app);
+	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
 
-	if (gprs && gc)
-		ofono_gprs_add_context(gprs, gc);
+	g_at_chat_send(data->mdm, "AT&C0", NULL, NULL, NULL, NULL);
+	g_at_chat_send(data->mdm, "AT&D2", NULL, NULL, NULL, NULL);
 
-	mw = ofono_message_waiting_create(modem);
-	if (mw)
-		ofono_message_waiting_register(mw);
+	/* Check for GSM capabilities */
+	g_at_chat_send(data->app, "AT+GCAP", gcap_prefix, gcap_support, modem,
+			NULL);
 }
 
 static struct ofono_modem_driver gemalto_driver = {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 1/1] gemalto: Improve init process
  2017-04-11  9:04 ` [PATCH 1/1] gemalto: " Vincent Cesson
@ 2017-04-11 12:30   ` Vincent Cesson
  2017-04-11 17:25     ` Denis Kenzior
  0 siblings, 1 reply; 15+ messages in thread
From: Vincent Cesson @ 2017-04-11 12:30 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 16870 bytes --]

Change Airplane mode: keep SIM connected (CFUN=4).
Fix enable/disable return value.
Create interfaces only if SIM card is detected.
Listen to URC for SIM status (improve SIM detection robustness).
Add some init commands.
---
Change in v2:
 - Add case in CFUN parsing

 plugins/gemalto.c | 470 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 434 insertions(+), 36 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index ffe6814..014bb4c 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -42,6 +42,7 @@
 #include <ofono/message-waiting.h>
 #include <ofono/netreg.h>
 #include <ofono/phonebook.h>
+#include <ofono/cbs.h>
 #include <ofono/sim.h>
 #include <ofono/sms.h>
 #include <ofono/ussd.h>
@@ -53,10 +54,27 @@
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
+static void at_cpin_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data);
+
+static const char *sind_prefix[] = { "^SIND:", NULL };
+static const char *creg_prefix[] = { "+CREG:", NULL };
+static const char *cpin_prefix[] = { "+CPIN:", NULL };
+static const char *cfun_prefix[] = { "+CFUN:", NULL };
+static const char *sdport_prefix[] = { "^SDPORT:", NULL };
+static const char *gcap_prefix[] = { "+GCAP:", NULL };
+
+static gboolean sim_iface_created = FALSE;
 
 struct gemalto_data {
 	GAtChat *app;
 	GAtChat *mdm;
+	struct ofono_sim *sim;
+	gboolean pin_required;
+	gboolean need_reboot;
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
+	gboolean have_gsm;
 };
 
 static int gemalto_probe(struct ofono_modem *modem)
@@ -67,6 +85,10 @@ static int gemalto_probe(struct ofono_modem *modem)
 	if (data == NULL)
 		return -ENOMEM;
 
+	sim_iface_created = FALSE;
+	data->sim = NULL;
+	data->have_gsm = FALSE;
+
 	ofono_modem_set_data(modem, data);
 
 	return 0;
@@ -110,6 +132,128 @@ static GAtChat *open_device(const char *device)
 	return chat;
 }
 
+static void gemalto_cfun_enable_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+
+	DBG("");
+
+	/* Power on the modem */
+	ofono_modem_set_powered(modem, ok);
+}
+
+gboolean gemalto_modem_set_to_online(struct ofono_modem *modem)
+{
+	DBG("");
+	ofono_modem_set_powered(modem, TRUE);
+
+	/* Must return FALSE to stop the g_timeout loop */
+	return FALSE;
+}
+
+static void gemalto_cfun_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	gint init_online_state = 0;
+
+	DBG("");
+
+	if (!ok) {
+		/* In case of errors, the mode is not powered */
+		ofono_modem_set_powered(modem, FALSE);
+	} else {
+		g_at_result_iter_init(&iter, result);
+		g_at_result_iter_next(&iter, "+CFUN:");
+		g_at_result_iter_next_number(&iter, &init_online_state);
+
+		DBG("online = %d", init_online_state);
+		switch (init_online_state) {
+		case 0:
+			/* Keep modem offline but connect USIM */
+			g_at_chat_send(data->app, "AT+CFUN=4", NULL,
+					gemalto_cfun_enable_cb, modem, NULL);
+			break;
+		case 1:
+			/* Wait for 300msec otherwise the sim manager has not finished
+			   and it is used in the NetworkManager.
+			   Otherwise, get a seg fault */
+			g_timeout_add(300, (GSourceFunc)gemalto_modem_set_to_online,
+					modem);
+			break;
+		case 4:
+			/* Set modem to powered */
+			ofono_modem_set_powered(modem, TRUE);
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+static void gemalto_sdport_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	gint sdport_mode = -1;
+
+	DBG("");
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+	g_at_result_iter_next(&iter, "^SDPORT:");
+	g_at_result_iter_next_number(&iter, &sdport_mode);
+
+	/* Already correct mode. */
+	if (sdport_mode == 3) {
+		/* Check the modem mode (online/offline) */
+		g_at_chat_send(data->app, "AT+CFUN?", cfun_prefix,
+				gemalto_cfun_query_cb, modem, NULL);
+	}
+}
+
+static void gemalto_smso_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	/* Try hard shutdown */
+	if (!ok) {
+		DBG("Error");
+		return;
+	}
+
+	g_at_chat_cancel_all(data->app);
+	g_at_chat_unregister_all(data->app);
+
+	g_at_chat_unref(data->mdm);
+	data->mdm = NULL;
+	g_at_chat_unref(data->app);
+	data->app = NULL;
+
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+void gemalto_shutdown(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	/* Shutdown the modem */
+	g_at_chat_send(data->app, "AT^SMSO", NULL, gemalto_smso_cb,
+			modem, NULL);
+}
+
 static int gemalto_enable(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
@@ -140,7 +284,24 @@ static int gemalto_enable(struct ofono_modem *modem)
 		g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
 	}
 
-	return 0;
+	g_at_chat_send(data->app, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
+
+	g_at_chat_send(data->app, "AT^SQPORT?", NULL, NULL, NULL, NULL);
+
+	/* TEMPORARY : Add this command to get data reconnection working */
+	g_at_chat_send(data->mdm, "AT&C0", NULL, NULL, NULL, NULL);
+
+	/* Configure the modem to be in profil 1 */
+	g_at_chat_send(data->app, "AT^SCFG=\"Radio/Mtpl\",1,1",
+			NULL, NULL, NULL, NULL);
+
+	g_at_chat_send(data->app, "AT^SDPORT?", sdport_prefix,
+			gemalto_sdport_query_cb, modem, NULL);
+
+	/* Add the Time Zone URC */
+	g_at_chat_send(data->app, "AT+CTZU=1", NULL, NULL, NULL, NULL);
+
+	return -EINPROGRESS;
 }
 
 static int gemalto_disable(struct ofono_modem *modem)
@@ -149,55 +310,272 @@ static int gemalto_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	g_at_chat_send(data->app, "AT^SMSO", NULL, NULL, NULL, NULL);
+	if (data->app == NULL)
+		return 0;
 
-	ofono_modem_set_data(modem, NULL);
+	gemalto_shutdown(modem);
 
-	return 0;
+	return -EINPROGRESS;
 }
 
-static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
+static void gemalto_cfun_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
 	ofono_modem_online_cb_t cb = cbd->cb;
-	struct ofono_error error;
-
-	decode_at_error(&error, g_at_result_final_response(result));
 
-	cb(&error, cbd->data);
+	if (ok)
+		CALLBACK_WITH_SUCCESS(cb, cbd->data);
+	else
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
 }
 
-static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
+static void gemalto_set_online(struct ofono_modem *modem, gboolean online,
 		ofono_modem_online_cb_t cb, void *user_data)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
 	struct cb_data *cbd = cb_data_new(cb, user_data);
-	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=0";
+	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=4";
 
 	DBG("modem %p %s", modem, online ? "online" : "offline");
 
-	if (g_at_chat_send(data->app, command, NULL, set_online_cb, cbd, g_free))
+	if (data->app == NULL) {
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		g_free(cbd);
+	}
+
+	g_at_chat_send(data->app, command, cfun_prefix, gemalto_cfun_cb, cbd,
+			g_free);
+}
+
+static void gemalto_create_sim(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	if (!sim_iface_created) {
+		sim_iface_created = TRUE;
+		/* Create the sim */
+		data->sim = ofono_sim_create(modem, 0, "atmodem",
+				data->app);
+		if (data->sim)
+			ofono_sim_inserted_notify(data->sim, TRUE);
+	}
+}
+
+static void gemalto_set_network(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	ofono_devinfo_create(modem, 0, "atmodem", data->app);
+
+	if (data->have_gsm == TRUE) {
+		ofono_sms_create(modem, 0, "atmodem", data->app);
+
+		ofono_cbs_create(modem, 0, "atmodem", data->app);
+
+		data->gprs = ofono_gprs_create(modem, 0,
+				"atmodem", data->app);
+
+		data->gc = ofono_gprs_context_create(modem, 0,
+				"atmodem", data->mdm);
+
+		if (data->gprs && data->gc)
+			ofono_gprs_add_context(data->gprs, data->gc);
+	}
+}
+
+static void gemalto_sind_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int value = 0;
+
+	DBG("");
+
+	if (ok) {
+		g_at_result_iter_init(&iter, result);
+		if (!g_at_result_iter_next(&iter, "^SIND:"))
+			return;
+		if (!g_at_result_iter_skip_next(&iter))
+			return;
+		if (!g_at_result_iter_skip_next(&iter))
+			return;
+		if (!g_at_result_iter_next_number(&iter, &value))
+			return;
+
+		if (value == 5) { /* SIM data ready */
+			/* If no pin required, we create the SIM interface */
+			if (!data->pin_required) {
+				/* Deactivate the SIND URC*/
+				g_at_chat_send(data->app, "AT^SIND=\"simstatus\",0",
+						sind_prefix, NULL, NULL, NULL);
+				gemalto_create_sim(modem);
+			}
+		}
+
+		g_at_result_iter_close_list(&iter);
+	}
+}
+
+static void gemalto_sind_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	char *signal_identifier = "simstatus";
+	const char *str;
+	int value = 0;
+
+	DBG("");
+
+	g_at_result_iter_init(&iter, result);
+	if (!g_at_result_iter_next(&iter, "+CIEV:"))
+		return;
+	if (!g_at_result_iter_next_unquoted_string(&iter, &str))
+		return;
+	if (g_str_equal(signal_identifier, str) == FALSE)
 		return;
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+	if (value == 5) { /* SIM data ready */
+		/* If no pin required, we create the SIM interface */
+		if (!data->pin_required)
+			gemalto_create_sim(modem);
+	}
+	g_at_result_iter_close_list(&iter);
+}
+
+static void gemalto_creg_query_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int value = 0;
+
+	DBG("");
+
+	if (ok) {
+		g_at_result_iter_init(&iter, result);
+		if (!g_at_result_iter_next(&iter, "+CREG:"))
+			return;
+		/* Skip the URC mode */
+		if (!g_at_result_iter_skip_next(&iter))
+			return;
+		if (!g_at_result_iter_next_number(&iter, &value))
+			return;
+		if (value == 5) {
+			/* Deactivate the CREG URC */
+			g_at_chat_send(data->app, "AT+CREG=0", NULL, NULL, NULL, NULL);
+
+			gemalto_set_network(modem);
+		}
+		g_at_result_iter_close_list(&iter);
+	}
+}
 
-	CALLBACK_WITH_FAILURE(cb, cbd->data);
+static void gemalto_creg_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int value = 0;
+
+	DBG("");
 
-	g_free(cbd);
+	g_at_result_iter_init(&iter, result);
+	if (!g_at_result_iter_next(&iter, "+CREG:"))
+		return;
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+
+	if (value == 5 && !data->have_gsm) {
+		data->have_gsm = TRUE;
+		gemalto_set_network(modem);
+	}
+	g_at_result_iter_close_list(&iter);
+}
+
+static gboolean at_pin_query(struct ofono_modem *modem)
+{
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	/* Invert FALSE = stop, TRUE = re-do to be used in g_timeout_add_seconds */
+	gboolean ret = FALSE;
+
+	if (g_at_chat_send(data->mdm, "AT+CPIN?", cpin_prefix,
+				at_cpin_query_cb, modem, NULL) <= 0) {
+		DBG("Error while querying the PIN.");
+		ret = TRUE;
+	}
+	return ret;
+}
+
+static void at_cpin_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	struct ofono_error error;
+	const char *pin_type_str;
+
+	if (ok) {
+		g_at_result_iter_init(&iter, result);
+
+		if (g_at_result_iter_next(&iter, "+CPIN:")) {
+			g_at_result_iter_next_unquoted_string(&iter, &pin_type_str);
+			/* No PIN required or there is an error */
+			if (g_strcmp0(pin_type_str, "READY") == 0) {
+				data->pin_required = FALSE;
+			} else { /* PIN is required */
+				data->pin_required = TRUE;
+				/* Create the SIM manager to enter PIN */
+				gemalto_create_sim(modem);
+			}
+		}
+	} else {
+		decode_at_error(&error, g_at_result_final_response(result));
+		/* If the error is CME busy */
+		if (error.type == OFONO_ERROR_TYPE_CME && error.error == 14) {
+			DBG("Invalid password => retry the command");
+			/* We re-try the command */
+			g_timeout_add_seconds(1, (GSourceFunc)at_pin_query, modem);
+			/* If the SIM is not inserted */
+		} else if (error.type == OFONO_ERROR_TYPE_CME && error.error == 11) {
+
+			DBG("PIN not entered, please enter PIN");
+			g_timeout_add_seconds(1, (GSourceFunc)at_pin_query, modem);
+
+		}
+	}
 }
 
 static void gemalto_pre_sim(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
-	struct ofono_sim *sim;
 
 	DBG("%p", modem);
 
 	ofono_devinfo_create(modem, 0, "atmodem", data->app);
-	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
-	ofono_voicecall_create(modem, 0, "atmodem", data->app);
 	ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
 
-	if (sim)
-		ofono_sim_inserted_notify(sim, TRUE);
+	/* Listen to simstatus SIND URC */
+	g_at_chat_register(data->app, "+CIEV:",
+			gemalto_sind_notify, FALSE, modem, NULL);
+	/* Listen to CREG URC */
+	g_at_chat_register(data->app, "+CREG:",
+			gemalto_creg_notify, FALSE, modem, NULL);
+
+	/* Check the SIM status and enable simstatus SIND URC */
+	g_at_chat_send(data->mdm, "AT^SIND=\"simstatus\",1", sind_prefix,
+			gemalto_sind_cb, modem, NULL);
+
+	/* Query the PIN command to create SIM Manager (or not) */
+	at_pin_query(modem);
 }
 
 static void gemalto_post_sim(struct ofono_modem *modem)
@@ -207,35 +585,55 @@ static void gemalto_post_sim(struct ofono_modem *modem)
 	DBG("%p", modem);
 
 	ofono_phonebook_create(modem, 0, "atmodem", data->app);
+}
+
+static void gcap_support(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	const char *gcap;
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+GCAP:"))
+		return;
 
-	ofono_sms_create(modem, 0, "atmodem", data->app);
+	while (g_at_result_iter_next_unquoted_string(&iter, &gcap)) {
+		if (*gcap == '\0')
+			break;
+
+		if (!g_strcmp0(gcap, "+CGSM"))
+			data->have_gsm = TRUE;
+	}
+
+	if (data->have_gsm)
+		gemalto_set_network(modem);
 }
 
 static void gemalto_post_online(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
-	struct ofono_message_waiting *mw;
-	struct ofono_gprs *gprs;
-	struct ofono_gprs_context *gc;
 
 	DBG("%p", modem);
 
-	ofono_ussd_create(modem, 0, "atmodem", data->app);
-	ofono_call_forwarding_create(modem, 0, "atmodem", data->app);
-	ofono_call_settings_create(modem, 0, "atmodem", data->app);
-	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
-	ofono_call_meter_create(modem, 0, "atmodem", data->app);
-	ofono_call_barring_create(modem, 0, "atmodem", data->app);
+	/* Enable CREG URC and check CREG status */
+	g_at_chat_send(data->app, "AT+CREG=1", creg_prefix, NULL, modem, NULL);
+	g_at_chat_send(data->app, "AT+CREG?", creg_prefix,
+			gemalto_creg_query_cb, modem, NULL);
 
-	gprs = ofono_gprs_create(modem, 0, "atmodem", data->app);
-	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
+	ofono_voicecall_create(modem, 0, "atmodem", data->app);
+	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
 
-	if (gprs && gc)
-		ofono_gprs_add_context(gprs, gc);
+	g_at_chat_send(data->mdm, "AT&C0", NULL, NULL, NULL, NULL);
+	g_at_chat_send(data->mdm, "AT&D2", NULL, NULL, NULL, NULL);
 
-	mw = ofono_message_waiting_create(modem);
-	if (mw)
-		ofono_message_waiting_register(mw);
+	/* Check for GSM capabilities */
+	g_at_chat_send(data->app, "AT+GCAP", gcap_prefix, gcap_support, modem,
+			NULL);
 }
 
 static struct ofono_modem_driver gemalto_driver = {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/1] gemalto: Improve init process
  2017-04-11 12:30   ` [PATCH v2 " Vincent Cesson
@ 2017-04-11 17:25     ` Denis Kenzior
  2017-04-13  8:05       ` [PATCH v3 0/3] " Vincent Cesson
  0 siblings, 1 reply; 15+ messages in thread
From: Denis Kenzior @ 2017-04-11 17:25 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 19073 bytes --]

Hi Vincent,

On 04/11/2017 07:30 AM, Vincent Cesson wrote:
> Change Airplane mode: keep SIM connected (CFUN=4).
> Fix enable/disable return value.
> Create interfaces only if SIM card is detected.
> Listen to URC for SIM status (improve SIM detection robustness).
> Add some init commands.
> ---
> Change in v2:
>  - Add case in CFUN parsing
>
>  plugins/gemalto.c | 470 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 434 insertions(+), 36 deletions(-)

Please consider splitting this into multiple commits.  Structured 
logically.  It will make things much easier to review and get your 
changes upstream faster.

Also, please review our coding style guidelines.  doc/coding-style.txt. 
There are many style issues in this submission.

>
> diff --git a/plugins/gemalto.c b/plugins/gemalto.c
> index ffe6814..014bb4c 100644
> --- a/plugins/gemalto.c
> +++ b/plugins/gemalto.c
> @@ -42,6 +42,7 @@
>  #include <ofono/message-waiting.h>
>  #include <ofono/netreg.h>
>  #include <ofono/phonebook.h>
> +#include <ofono/cbs.h>
>  #include <ofono/sim.h>
>  #include <ofono/sms.h>
>  #include <ofono/ussd.h>
> @@ -53,10 +54,27 @@
>  #include <drivers/atmodem/atutil.h>
>  #include <drivers/atmodem/vendor.h>
>
> +static void at_cpin_query_cb(gboolean ok, GAtResult *result,
> +		gpointer user_data);
> +
> +static const char *sind_prefix[] = { "^SIND:", NULL };
> +static const char *creg_prefix[] = { "+CREG:", NULL };
> +static const char *cpin_prefix[] = { "+CPIN:", NULL };
> +static const char *cfun_prefix[] = { "+CFUN:", NULL };
> +static const char *sdport_prefix[] = { "^SDPORT:", NULL };
> +static const char *gcap_prefix[] = { "+GCAP:", NULL };
> +
> +static gboolean sim_iface_created = FALSE;

Nope.  You can't do that.  This is a global variable and we might 
potentially have multiple instances of this modem at the same time.
>
>  struct gemalto_data {
>  	GAtChat *app;
>  	GAtChat *mdm;
> +	struct ofono_sim *sim;
> +	gboolean pin_required;
> +	gboolean need_reboot;
> +	struct ofono_gprs *gprs;
> +	struct ofono_gprs_context *gc;
> +	gboolean have_gsm;
>  };
>
>  static int gemalto_probe(struct ofono_modem *modem)
> @@ -67,6 +85,10 @@ static int gemalto_probe(struct ofono_modem *modem)
>  	if (data == NULL)
>  		return -ENOMEM;
>
> +	sim_iface_created = FALSE;
> +	data->sim = NULL;
> +	data->have_gsm = FALSE;
> +
>  	ofono_modem_set_data(modem, data);
>
>  	return 0;
> @@ -110,6 +132,128 @@ static GAtChat *open_device(const char *device)
>  	return chat;
>  }
>
> +static void gemalto_cfun_enable_cb(gboolean ok, GAtResult *result,
> +		gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +
> +	DBG("");
> +
> +	/* Power on the modem */
> +	ofono_modem_set_powered(modem, ok);
> +}
> +
> +gboolean gemalto_modem_set_to_online(struct ofono_modem *modem)
> +{
> +	DBG("");
> +	ofono_modem_set_powered(modem, TRUE);
> +
> +	/* Must return FALSE to stop the g_timeout loop */
> +	return FALSE;
> +}
> +
> +static void gemalto_cfun_query_cb(gboolean ok, GAtResult *result,
> +		gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	gint init_online_state = 0;
> +
> +	DBG("");
> +
> +	if (!ok) {
> +		/* In case of errors, the mode is not powered */
> +		ofono_modem_set_powered(modem, FALSE);
> +	} else {
> +		g_at_result_iter_init(&iter, result);
> +		g_at_result_iter_next(&iter, "+CFUN:");
> +		g_at_result_iter_next_number(&iter, &init_online_state);
> +
> +		DBG("online = %d", init_online_state);
> +		switch (init_online_state) {
> +		case 0:
> +			/* Keep modem offline but connect USIM */
> +			g_at_chat_send(data->app, "AT+CFUN=4", NULL,
> +					gemalto_cfun_enable_cb, modem, NULL);
> +			break;
> +		case 1:
> +			/* Wait for 300msec otherwise the sim manager has not finished
> +			   and it is used in the NetworkManager.
> +			   Otherwise, get a seg fault */
> +			g_timeout_add(300, (GSourceFunc)gemalto_modem_set_to_online,
> +					modem);
> +			break;
> +		case 4:
> +			/* Set modem to powered */
> +			ofono_modem_set_powered(modem, TRUE);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
> +static void gemalto_sdport_query_cb(gboolean ok, GAtResult *result,
> +		gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	gint sdport_mode = -1;
> +
> +	DBG("");
> +
> +	if (!ok)
> +		return;
> +
> +	g_at_result_iter_init(&iter, result);
> +	g_at_result_iter_next(&iter, "^SDPORT:");
> +	g_at_result_iter_next_number(&iter, &sdport_mode);
> +
> +	/* Already correct mode. */
> +	if (sdport_mode == 3) {
> +		/* Check the modem mode (online/offline) */
> +		g_at_chat_send(data->app, "AT+CFUN?", cfun_prefix,
> +				gemalto_cfun_query_cb, modem, NULL);
> +	}
> +}
> +
> +static void gemalto_smso_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	/* Try hard shutdown */
> +	if (!ok) {
> +		DBG("Error");
> +		return;
> +	}
> +
> +	g_at_chat_cancel_all(data->app);
> +	g_at_chat_unregister_all(data->app);
> +
> +	g_at_chat_unref(data->mdm);
> +	data->mdm = NULL;
> +	g_at_chat_unref(data->app);
> +	data->app = NULL;
> +
> +	ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +void gemalto_shutdown(struct ofono_modem *modem)
> +{
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	/* Shutdown the modem */
> +	g_at_chat_send(data->app, "AT^SMSO", NULL, gemalto_smso_cb,
> +			modem, NULL);
> +}
> +
>  static int gemalto_enable(struct ofono_modem *modem)
>  {
>  	struct gemalto_data *data = ofono_modem_get_data(modem);
> @@ -140,7 +284,24 @@ static int gemalto_enable(struct ofono_modem *modem)
>  		g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
>  	}
>
> -	return 0;
> +	g_at_chat_send(data->app, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
> +
> +	g_at_chat_send(data->app, "AT^SQPORT?", NULL, NULL, NULL, NULL);
> +
> +	/* TEMPORARY : Add this command to get data reconnection working */
> +	g_at_chat_send(data->mdm, "AT&C0", NULL, NULL, NULL, NULL);
> +
> +	/* Configure the modem to be in profil 1 */
> +	g_at_chat_send(data->app, "AT^SCFG=\"Radio/Mtpl\",1,1",
> +			NULL, NULL, NULL, NULL);
> +
> +	g_at_chat_send(data->app, "AT^SDPORT?", sdport_prefix,
> +			gemalto_sdport_query_cb, modem, NULL);
> +
> +	/* Add the Time Zone URC */
> +	g_at_chat_send(data->app, "AT+CTZU=1", NULL, NULL, NULL, NULL);

This belongs in the netreg driver.

> +
> +	return -EINPROGRESS;
>  }
>
>  static int gemalto_disable(struct ofono_modem *modem)
> @@ -149,55 +310,272 @@ static int gemalto_disable(struct ofono_modem *modem)
>
>  	DBG("%p", modem);
>
> -	g_at_chat_send(data->app, "AT^SMSO", NULL, NULL, NULL, NULL);
> +	if (data->app == NULL)
> +		return 0;
>
> -	ofono_modem_set_data(modem, NULL);
> +	gemalto_shutdown(modem);
>
> -	return 0;
> +	return -EINPROGRESS;
>  }
>
> -static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +static void gemalto_cfun_cb(gboolean ok, GAtResult *result, gpointer user_data)
>  {
>  	struct cb_data *cbd = user_data;
>  	ofono_modem_online_cb_t cb = cbd->cb;
> -	struct ofono_error error;
> -
> -	decode_at_error(&error, g_at_result_final_response(result));
>
> -	cb(&error, cbd->data);
> +	if (ok)
> +		CALLBACK_WITH_SUCCESS(cb, cbd->data);
> +	else
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);
>  }
>
> -static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
> +static void gemalto_set_online(struct ofono_modem *modem, gboolean online,
>  		ofono_modem_online_cb_t cb, void *user_data)
>  {
>  	struct gemalto_data *data = ofono_modem_get_data(modem);
>  	struct cb_data *cbd = cb_data_new(cb, user_data);
> -	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=0";
> +	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=4";
>
>  	DBG("modem %p %s", modem, online ? "online" : "offline");
>
> -	if (g_at_chat_send(data->app, command, NULL, set_online_cb, cbd, g_free))
> +	if (data->app == NULL) {
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);
> +		g_free(cbd);
> +	}
> +
> +	g_at_chat_send(data->app, command, cfun_prefix, gemalto_cfun_cb, cbd,
> +			g_free);
> +}
> +
> +static void gemalto_create_sim(struct ofono_modem *modem)
> +{
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	if (!sim_iface_created) {
> +		sim_iface_created = TRUE;
> +		/* Create the sim */
> +		data->sim = ofono_sim_create(modem, 0, "atmodem",
> +				data->app);
> +		if (data->sim)
> +			ofono_sim_inserted_notify(data->sim, TRUE);
> +	}
> +}
> +
> +static void gemalto_set_network(struct ofono_modem *modem)
> +{
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	ofono_devinfo_create(modem, 0, "atmodem", data->app);
> +
> +	if (data->have_gsm == TRUE) {
> +		ofono_sms_create(modem, 0, "atmodem", data->app);
> +
> +		ofono_cbs_create(modem, 0, "atmodem", data->app);
> +
> +		data->gprs = ofono_gprs_create(modem, 0,
> +				"atmodem", data->app);
> +
> +		data->gc = ofono_gprs_context_create(modem, 0,
> +				"atmodem", data->mdm);
> +
> +		if (data->gprs && data->gc)
> +			ofono_gprs_add_context(data->gprs, data->gc);
> +	}
> +}
> +
> +static void gemalto_sind_cb(gboolean ok, GAtResult *result,
> +		gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	int value = 0;
> +
> +	DBG("");
> +
> +	if (ok) {
> +		g_at_result_iter_init(&iter, result);
> +		if (!g_at_result_iter_next(&iter, "^SIND:"))
> +			return;
> +		if (!g_at_result_iter_skip_next(&iter))
> +			return;
> +		if (!g_at_result_iter_skip_next(&iter))
> +			return;
> +		if (!g_at_result_iter_next_number(&iter, &value))
> +			return;
> +
> +		if (value == 5) { /* SIM data ready */
> +			/* If no pin required, we create the SIM interface */
> +			if (!data->pin_required) {
> +				/* Deactivate the SIND URC*/
> +				g_at_chat_send(data->app, "AT^SIND=\"simstatus\",0",
> +						sind_prefix, NULL, NULL, NULL);
> +				gemalto_create_sim(modem);
> +			}
> +		}
> +
> +		g_at_result_iter_close_list(&iter);
> +	}
> +}
> +
> +static void gemalto_sind_notify(GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	char *signal_identifier = "simstatus";
> +	const char *str;
> +	int value = 0;
> +
> +	DBG("");
> +
> +	g_at_result_iter_init(&iter, result);
> +	if (!g_at_result_iter_next(&iter, "+CIEV:"))
> +		return;
> +	if (!g_at_result_iter_next_unquoted_string(&iter, &str))
> +		return;
> +	if (g_str_equal(signal_identifier, str) == FALSE)
>  		return;
> +	if (!g_at_result_iter_next_number(&iter, &value))
> +		return;
> +	if (value == 5) { /* SIM data ready */
> +		/* If no pin required, we create the SIM interface */
> +		if (!data->pin_required)
> +			gemalto_create_sim(modem);
> +	}
> +	g_at_result_iter_close_list(&iter);

Lots of style violations for doc/coding-style.txt item M1

> +}
> +
> +static void gemalto_creg_query_cb(gboolean ok, GAtResult *result,
> +		gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	int value = 0;
> +
> +	DBG("");
> +
> +	if (ok) {
> +		g_at_result_iter_init(&iter, result);
> +		if (!g_at_result_iter_next(&iter, "+CREG:"))
> +			return;
> +		/* Skip the URC mode */
> +		if (!g_at_result_iter_skip_next(&iter))
> +			return;
> +		if (!g_at_result_iter_next_number(&iter, &value))
> +			return;
> +		if (value == 5) {
> +			/* Deactivate the CREG URC */
> +			g_at_chat_send(data->app, "AT+CREG=0", NULL, NULL, NULL, NULL);
> +
> +			gemalto_set_network(modem);
> +		}
> +		g_at_result_iter_close_list(&iter);

item M1

Why are you dealing with network registration inside the modem driver? 
Atoms are created based whether the radio is on or off and not whether 
you're registered/roaming

> +	}
> +}
>
> -	CALLBACK_WITH_FAILURE(cb, cbd->data);
> +static void gemalto_creg_notify(GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	int value = 0;
> +
> +	DBG("");
>
> -	g_free(cbd);
> +	g_at_result_iter_init(&iter, result);
> +	if (!g_at_result_iter_next(&iter, "+CREG:"))
> +		return;
> +	if (!g_at_result_iter_next_number(&iter, &value))
> +		return;
> +
> +	if (value == 5 && !data->have_gsm) {
> +		data->have_gsm = TRUE;
> +		gemalto_set_network(modem);
> +	}
> +	g_at_result_iter_close_list(&iter);
> +}
> +
> +static gboolean at_pin_query(struct ofono_modem *modem)
> +{
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	/* Invert FALSE = stop, TRUE = re-do to be used in g_timeout_add_seconds */
> +	gboolean ret = FALSE;
> +
> +	if (g_at_chat_send(data->mdm, "AT+CPIN?", cpin_prefix,
> +				at_cpin_query_cb, modem, NULL) <= 0) {
> +		DBG("Error while querying the PIN.");
> +		ret = TRUE;
> +	}
> +	return ret;
> +}
> +
> +static void at_cpin_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	struct ofono_error error;
> +	const char *pin_type_str;
> +
> +	if (ok) {
> +		g_at_result_iter_init(&iter, result);
> +
> +		if (g_at_result_iter_next(&iter, "+CPIN:")) {
> +			g_at_result_iter_next_unquoted_string(&iter, &pin_type_str);
> +			/* No PIN required or there is an error */
> +			if (g_strcmp0(pin_type_str, "READY") == 0) {
> +				data->pin_required = FALSE;
> +			} else { /* PIN is required */
> +				data->pin_required = TRUE;
> +				/* Create the SIM manager to enter PIN */
> +				gemalto_create_sim(modem);
> +			}
> +		}
> +	} else {
> +		decode_at_error(&error, g_at_result_final_response(result));
> +		/* If the error is CME busy */
> +		if (error.type == OFONO_ERROR_TYPE_CME && error.error == 14) {
> +			DBG("Invalid password => retry the command");
> +			/* We re-try the command */
> +			g_timeout_add_seconds(1, (GSourceFunc)at_pin_query, modem);
> +			/* If the SIM is not inserted */
> +		} else if (error.type == OFONO_ERROR_TYPE_CME && error.error == 11) {
> +
> +			DBG("PIN not entered, please enter PIN");
> +			g_timeout_add_seconds(1, (GSourceFunc)at_pin_query, modem);
> +
> +		}
> +	}

What are you trying to do here?  If this is one of those modems without 
a proper sim inserted notification, then you might want to use 
at_util_sim_state_query_new instead of re-inventing the wheel.

>  }
>
>  static void gemalto_pre_sim(struct ofono_modem *modem)
>  {
>  	struct gemalto_data *data = ofono_modem_get_data(modem);
> -	struct ofono_sim *sim;
>
>  	DBG("%p", modem);
>
>  	ofono_devinfo_create(modem, 0, "atmodem", data->app);
> -	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
> -	ofono_voicecall_create(modem, 0, "atmodem", data->app);
>  	ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
>
> -	if (sim)
> -		ofono_sim_inserted_notify(sim, TRUE);
> +	/* Listen to simstatus SIND URC */
> +	g_at_chat_register(data->app, "+CIEV:",
> +			gemalto_sind_notify, FALSE, modem, NULL);
> +	/* Listen to CREG URC */
> +	g_at_chat_register(data->app, "+CREG:",
> +			gemalto_creg_notify, FALSE, modem, NULL);
> +
> +	/* Check the SIM status and enable simstatus SIND URC */
> +	g_at_chat_send(data->mdm, "AT^SIND=\"simstatus\",1", sind_prefix,
> +			gemalto_sind_cb, modem, NULL);
> +
> +	/* Query the PIN command to create SIM Manager (or not) */
> +	at_pin_query(modem);

No AT commands inside pre_sim, post_sim or post_online please.  That is 
not what they're meant for.

>  }
>
>  static void gemalto_post_sim(struct ofono_modem *modem)
> @@ -207,35 +585,55 @@ static void gemalto_post_sim(struct ofono_modem *modem)
>  	DBG("%p", modem);
>
>  	ofono_phonebook_create(modem, 0, "atmodem", data->app);
> +}
> +
> +static void gcap_support(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	const char *gcap;
> +
> +	if (!ok)
> +		return;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "+GCAP:"))
> +		return;
>
> -	ofono_sms_create(modem, 0, "atmodem", data->app);
> +	while (g_at_result_iter_next_unquoted_string(&iter, &gcap)) {
> +		if (*gcap == '\0')
> +			break;
> +
> +		if (!g_strcmp0(gcap, "+CGSM"))
> +			data->have_gsm = TRUE;
> +	}
> +
> +	if (data->have_gsm)
> +		gemalto_set_network(modem);
>  }
>
>  static void gemalto_post_online(struct ofono_modem *modem)
>  {
>  	struct gemalto_data *data = ofono_modem_get_data(modem);
> -	struct ofono_message_waiting *mw;
> -	struct ofono_gprs *gprs;
> -	struct ofono_gprs_context *gc;
>
>  	DBG("%p", modem);
>
> -	ofono_ussd_create(modem, 0, "atmodem", data->app);
> -	ofono_call_forwarding_create(modem, 0, "atmodem", data->app);
> -	ofono_call_settings_create(modem, 0, "atmodem", data->app);
> -	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
> -	ofono_call_meter_create(modem, 0, "atmodem", data->app);
> -	ofono_call_barring_create(modem, 0, "atmodem", data->app);
> +	/* Enable CREG URC and check CREG status */
> +	g_at_chat_send(data->app, "AT+CREG=1", creg_prefix, NULL, modem, NULL);
> +	g_at_chat_send(data->app, "AT+CREG?", creg_prefix,
> +			gemalto_creg_query_cb, modem, NULL);
>
> -	gprs = ofono_gprs_create(modem, 0, "atmodem", data->app);
> -	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
> +	ofono_voicecall_create(modem, 0, "atmodem", data->app);
> +	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
>
> -	if (gprs && gc)
> -		ofono_gprs_add_context(gprs, gc);
> +	g_at_chat_send(data->mdm, "AT&C0", NULL, NULL, NULL, NULL);
> +	g_at_chat_send(data->mdm, "AT&D2", NULL, NULL, NULL, NULL);
>
> -	mw = ofono_message_waiting_create(modem);
> -	if (mw)
> -		ofono_message_waiting_register(mw);
> +	/* Check for GSM capabilities */
> +	g_at_chat_send(data->app, "AT+GCAP", gcap_prefix, gcap_support, modem,
> +			NULL);
>  }
>
>  static struct ofono_modem_driver gemalto_driver = {
>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 0/3] gemalto: Improve init process
  2017-04-11 17:25     ` Denis Kenzior
@ 2017-04-13  8:05       ` Vincent Cesson
  2017-04-13  8:05         ` [PATCH v3 1/3] gemalto: Clean post init functions Vincent Cesson
                           ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Vincent Cesson @ 2017-04-13  8:05 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Set of three patches improving Gemalto init. 
Denis' comments taken into account, I use sim_state_query to handle 
sim detection.

First patch cleans unused/untested interfaces.
Second patch fixes airplane mode.
Third patch improves sim detection with use of sim_state_query.

Changes in v3:
 - Replace URCs/callback mess with sim_state_query
 - Fix coding style

Vincent Cesson (3):
  gemalto: Clean post init functions
  gemalto: Change offline mode, keep USIM available
  gemalto: Use sim_state_query for sim detection

 plugins/gemalto.c | 125 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 36 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/3] gemalto: Clean post init functions
  2017-04-13  8:05       ` [PATCH v3 0/3] " Vincent Cesson
@ 2017-04-13  8:05         ` Vincent Cesson
  2017-04-13 16:56           ` Denis Kenzior
  2017-04-13  8:05         ` [PATCH v3 2/3] gemalto: Change offline mode, keep USIM available Vincent Cesson
  2017-04-13  8:05         ` [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection Vincent Cesson
  2 siblings, 1 reply; 15+ messages in thread
From: Vincent Cesson @ 2017-04-13  8:05 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2843 bytes --]

Remove inappropriate interfaces.
Move GPRS init from post_online to post_sim.
---
 plugins/gemalto.c | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index ffe6814..846e263 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -34,18 +34,11 @@
 #include <ofono/plugin.h>
 #include <ofono/log.h>
 #include <ofono/modem.h>
-#include <ofono/call-barring.h>
-#include <ofono/call-forwarding.h>
-#include <ofono/call-meter.h>
-#include <ofono/call-settings.h>
 #include <ofono/devinfo.h>
-#include <ofono/message-waiting.h>
 #include <ofono/netreg.h>
 #include <ofono/phonebook.h>
 #include <ofono/sim.h>
 #include <ofono/sms.h>
-#include <ofono/ussd.h>
-#include <ofono/voicecall.h>
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
 #include <ofono/location-reporting.h>
@@ -193,7 +186,6 @@ static void gemalto_pre_sim(struct ofono_modem *modem)
 
 	ofono_devinfo_create(modem, 0, "atmodem", data->app);
 	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
-	ofono_voicecall_create(modem, 0, "atmodem", data->app);
 	ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
 
 	if (sim)
@@ -203,39 +195,29 @@ static void gemalto_pre_sim(struct ofono_modem *modem)
 static void gemalto_post_sim(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
+	struct ofono_gprs *gprs;
+	struct ofono_gprs_context *gc;
 
 	DBG("%p", modem);
 
 	ofono_phonebook_create(modem, 0, "atmodem", data->app);
 
 	ofono_sms_create(modem, 0, "atmodem", data->app);
+
+	gprs = ofono_gprs_create(modem, 0, "atmodem", data->app);
+	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
+
+	if (gprs && gc)
+		ofono_gprs_add_context(gprs, gc);
 }
 
 static void gemalto_post_online(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
-	struct ofono_message_waiting *mw;
-	struct ofono_gprs *gprs;
-	struct ofono_gprs_context *gc;
 
 	DBG("%p", modem);
 
-	ofono_ussd_create(modem, 0, "atmodem", data->app);
-	ofono_call_forwarding_create(modem, 0, "atmodem", data->app);
-	ofono_call_settings_create(modem, 0, "atmodem", data->app);
 	ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
-	ofono_call_meter_create(modem, 0, "atmodem", data->app);
-	ofono_call_barring_create(modem, 0, "atmodem", data->app);
-
-	gprs = ofono_gprs_create(modem, 0, "atmodem", data->app);
-	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
-
-	if (gprs && gc)
-		ofono_gprs_add_context(gprs, gc);
-
-	mw = ofono_message_waiting_create(modem);
-	if (mw)
-		ofono_message_waiting_register(mw);
 }
 
 static struct ofono_modem_driver gemalto_driver = {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/3] gemalto: Change offline mode, keep USIM available
  2017-04-13  8:05       ` [PATCH v3 0/3] " Vincent Cesson
  2017-04-13  8:05         ` [PATCH v3 1/3] gemalto: Clean post init functions Vincent Cesson
@ 2017-04-13  8:05         ` Vincent Cesson
  2017-04-13 16:56           ` Denis Kenzior
  2017-04-13  8:05         ` [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection Vincent Cesson
  2 siblings, 1 reply; 15+ messages in thread
From: Vincent Cesson @ 2017-04-13  8:05 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

Gemalto has two airplane mode:
CFUN=0 disables USIM
CFUN=4 keeps USIM connected
---
 plugins/gemalto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index 846e263..ab0da8f 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -165,7 +165,7 @@ static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
 	struct cb_data *cbd = cb_data_new(cb, user_data);
-	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=0";
+	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=4";
 
 	DBG("modem %p %s", modem, online ? "online" : "offline");
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection
  2017-04-13  8:05       ` [PATCH v3 0/3] " Vincent Cesson
  2017-04-13  8:05         ` [PATCH v3 1/3] gemalto: Clean post init functions Vincent Cesson
  2017-04-13  8:05         ` [PATCH v3 2/3] gemalto: Change offline mode, keep USIM available Vincent Cesson
@ 2017-04-13  8:05         ` Vincent Cesson
  2017-04-13 17:07           ` Denis Kenzior
  2 siblings, 1 reply; 15+ messages in thread
From: Vincent Cesson @ 2017-04-13  8:05 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 5550 bytes --]

Populate gemalto_data structure.
Add sim_state callbacks.
Fix enable/disable return value.
---
 plugins/gemalto.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 80 insertions(+), 9 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index ab0da8f..5473d2f 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -46,10 +46,14 @@
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
+static const char *none_prefix[] = { NULL };
 
 struct gemalto_data {
 	GAtChat *app;
 	GAtChat *mdm;
+	struct ofono_sim *sim;
+	gboolean have_sim;
+	struct at_util_sim_state_query *sim_state_query;
 };
 
 static int gemalto_probe(struct ofono_modem *modem)
@@ -57,9 +61,12 @@ static int gemalto_probe(struct ofono_modem *modem)
 	struct gemalto_data *data;
 
 	data = g_try_new0(struct gemalto_data, 1);
+
 	if (data == NULL)
 		return -ENOMEM;
 
+	data->sim = NULL;
+
 	ofono_modem_set_data(modem, data);
 
 	return 0;
@@ -69,6 +76,8 @@ static void gemalto_remove(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
 
+	/* Cleanup potential SIM state polling */
+	at_util_sim_state_query_free(data->sim_state_query);
 	ofono_modem_set_data(modem, NULL);
 	g_free(data);
 }
@@ -89,6 +98,7 @@ static GAtChat *open_device(const char *device)
 	DBG("Opening device %s", device);
 
 	channel = g_at_tty_open(device, NULL);
+
 	if (channel == NULL)
 		return NULL;
 
@@ -103,6 +113,39 @@ static GAtChat *open_device(const char *device)
 	return chat;
 }
 
+static void sim_state_cb(gboolean present, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	at_util_sim_state_query_free(data->sim_state_query);
+	data->sim_state_query = NULL;
+
+	data->have_sim = present;
+	ofono_modem_set_powered(modem, TRUE);
+}
+
+static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	if (!ok) {
+		g_at_chat_unref(data->app);
+		data->app = NULL;
+
+		g_at_chat_unref(data->mdm);
+		data->mdm = NULL;
+
+		ofono_modem_set_powered(modem, FALSE);
+		return;
+	}
+
+	data->sim_state_query = at_util_sim_state_query_new(data->app,
+						2, 20, sim_state_cb, modem,
+						NULL);
+}
+
 static int gemalto_enable(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
@@ -118,10 +161,12 @@ static int gemalto_enable(struct ofono_modem *modem)
 
 	/* Open devices */
 	data->app = open_device(app);
+
 	if (data->app == NULL)
 		return -EINVAL;
 
 	data->mdm = open_device(mdm);
+
 	if (data->mdm == NULL) {
 		g_at_chat_unref(data->app);
 		data->app = NULL;
@@ -133,7 +178,32 @@ static int gemalto_enable(struct ofono_modem *modem)
 		g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
 	}
 
-	return 0;
+	g_at_chat_send(data->mdm, "ATE0", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(data->app, "ATE0 +CMEE=1", none_prefix,
+			NULL, NULL, NULL);
+	g_at_chat_send(data->mdm, "AT&C0", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(data->app, "AT&C0", none_prefix, NULL, NULL, NULL);
+
+	g_at_chat_send(data->app, "AT+CFUN=4", none_prefix,
+			cfun_enable, modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+static void gemalto_smso_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	g_at_chat_unref(data->mdm);
+	data->mdm = NULL;
+	g_at_chat_unref(data->app);
+	data->app = NULL;
+
+	if (ok)
+		ofono_modem_set_powered(modem, FALSE);
 }
 
 static int gemalto_disable(struct ofono_modem *modem)
@@ -142,11 +212,14 @@ static int gemalto_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	g_at_chat_send(data->app, "AT^SMSO", NULL, NULL, NULL, NULL);
+	g_at_chat_cancel_all(data->app);
+	g_at_chat_unregister_all(data->app);
 
-	ofono_modem_set_data(modem, NULL);
+	/* Shutdown the modem */
+	g_at_chat_send(data->app, "AT^SMSO", none_prefix, gemalto_smso_cb,
+			modem, NULL);
 
-	return 0;
+	return -EINPROGRESS;
 }
 
 static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -156,11 +229,10 @@ static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_error error;
 
 	decode_at_error(&error, g_at_result_final_response(result));
-
 	cb(&error, cbd->data);
 }
 
-static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
+static void gemalto_set_online(struct ofono_modem *modem, gboolean online,
 		ofono_modem_online_cb_t cb, void *user_data)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
@@ -173,7 +245,6 @@ static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
 		return;
 
 	CALLBACK_WITH_FAILURE(cb, cbd->data);
-
 	g_free(cbd);
 }
 
@@ -185,10 +256,10 @@ static void gemalto_pre_sim(struct ofono_modem *modem)
 	DBG("%p", modem);
 
 	ofono_devinfo_create(modem, 0, "atmodem", data->app);
-	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
 	ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
+	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
 
-	if (sim)
+	if (sim && data->have_sim == TRUE)
 		ofono_sim_inserted_notify(sim, TRUE);
 }
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/3] gemalto: Clean post init functions
  2017-04-13  8:05         ` [PATCH v3 1/3] gemalto: Clean post init functions Vincent Cesson
@ 2017-04-13 16:56           ` Denis Kenzior
  0 siblings, 0 replies; 15+ messages in thread
From: Denis Kenzior @ 2017-04-13 16:56 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

Hi Vincent,

On 04/13/2017 03:05 AM, Vincent Cesson wrote:
> Remove inappropriate interfaces.
> Move GPRS init from post_online to post_sim.
> ---
>  plugins/gemalto.c | 34 ++++++++--------------------------
>  1 file changed, 8 insertions(+), 26 deletions(-)
>

Applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] gemalto: Change offline mode, keep USIM available
  2017-04-13  8:05         ` [PATCH v3 2/3] gemalto: Change offline mode, keep USIM available Vincent Cesson
@ 2017-04-13 16:56           ` Denis Kenzior
  0 siblings, 0 replies; 15+ messages in thread
From: Denis Kenzior @ 2017-04-13 16:56 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

Hi Vincent,

On 04/13/2017 03:05 AM, Vincent Cesson wrote:
> Gemalto has two airplane mode:
> CFUN=0 disables USIM
> CFUN=4 keeps USIM connected
> ---
>  plugins/gemalto.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection
  2017-04-13  8:05         ` [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection Vincent Cesson
@ 2017-04-13 17:07           ` Denis Kenzior
  2017-04-18  8:38             ` Vincent CESSON
  0 siblings, 1 reply; 15+ messages in thread
From: Denis Kenzior @ 2017-04-13 17:07 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 6811 bytes --]

Hi Vincent,

On 04/13/2017 03:05 AM, Vincent Cesson wrote:
> Populate gemalto_data structure.
> Add sim_state callbacks.
> Fix enable/disable return value.
> ---
>  plugins/gemalto.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 80 insertions(+), 9 deletions(-)
>
> diff --git a/plugins/gemalto.c b/plugins/gemalto.c
> index ab0da8f..5473d2f 100644
> --- a/plugins/gemalto.c
> +++ b/plugins/gemalto.c
> @@ -46,10 +46,14 @@
>  #include <drivers/atmodem/atutil.h>
>  #include <drivers/atmodem/vendor.h>
>
> +static const char *none_prefix[] = { NULL };
>
>  struct gemalto_data {
>  	GAtChat *app;
>  	GAtChat *mdm;
> +	struct ofono_sim *sim;
> +	gboolean have_sim;
> +	struct at_util_sim_state_query *sim_state_query;
>  };
>
>  static int gemalto_probe(struct ofono_modem *modem)
> @@ -57,9 +61,12 @@ static int gemalto_probe(struct ofono_modem *modem)
>  	struct gemalto_data *data;
>
>  	data = g_try_new0(struct gemalto_data, 1);
> +

Strictly speaking you don't need an empty line here. 
doc/coding-style.txt item M1 mentions an exception at the end of the 
relevant section text.

I don't care what style variation you use, but please don't mix style 
fixes with code additions in the same patch.

>  	if (data == NULL)
>  		return -ENOMEM;
>
> +	data->sim = NULL;
> +

This isn't needed, g_try_new0 already memsets the allocated structure to 0.

>  	ofono_modem_set_data(modem, data);
>
>  	return 0;
> @@ -69,6 +76,8 @@ static void gemalto_remove(struct ofono_modem *modem)
>  {
>  	struct gemalto_data *data = ofono_modem_get_data(modem);
>
> +	/* Cleanup potential SIM state polling */
> +	at_util_sim_state_query_free(data->sim_state_query);
>  	ofono_modem_set_data(modem, NULL);
>  	g_free(data);
>  }
> @@ -89,6 +98,7 @@ static GAtChat *open_device(const char *device)
>  	DBG("Opening device %s", device);
>
>  	channel = g_at_tty_open(device, NULL);
> +

Again, style fix and not relevant to this commit

>  	if (channel == NULL)
>  		return NULL;
>
> @@ -103,6 +113,39 @@ static GAtChat *open_device(const char *device)
>  	return chat;
>  }
>
> +static void sim_state_cb(gboolean present, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +
> +	at_util_sim_state_query_free(data->sim_state_query);
> +	data->sim_state_query = NULL;
> +
> +	data->have_sim = present;
> +	ofono_modem_set_powered(modem, TRUE);
> +}
> +
> +static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +
> +	if (!ok) {
> +		g_at_chat_unref(data->app);
> +		data->app = NULL;
> +
> +		g_at_chat_unref(data->mdm);
> +		data->mdm = NULL;
> +
> +		ofono_modem_set_powered(modem, FALSE);
> +		return;
> +	}
> +
> +	data->sim_state_query = at_util_sim_state_query_new(data->app,
> +						2, 20, sim_state_cb, modem,
> +						NULL);
> +}
> +
>  static int gemalto_enable(struct ofono_modem *modem)
>  {
>  	struct gemalto_data *data = ofono_modem_get_data(modem);
> @@ -118,10 +161,12 @@ static int gemalto_enable(struct ofono_modem *modem)
>
>  	/* Open devices */
>  	data->app = open_device(app);
> +

See comments about style fixes

>  	if (data->app == NULL)
>  		return -EINVAL;
>
>  	data->mdm = open_device(mdm);
> +

See comments about style fixes

>  	if (data->mdm == NULL) {
>  		g_at_chat_unref(data->app);
>  		data->app = NULL;
> @@ -133,7 +178,32 @@ static int gemalto_enable(struct ofono_modem *modem)
>  		g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
>  	}
>
> -	return 0;
> +	g_at_chat_send(data->mdm, "ATE0", none_prefix, NULL, NULL, NULL);
> +	g_at_chat_send(data->app, "ATE0 +CMEE=1", none_prefix,
> +			NULL, NULL, NULL);
> +	g_at_chat_send(data->mdm, "AT&C0", none_prefix, NULL, NULL, NULL);
> +	g_at_chat_send(data->app, "AT&C0", none_prefix, NULL, NULL, NULL);
> +
> +	g_at_chat_send(data->app, "AT+CFUN=4", none_prefix,
> +			cfun_enable, modem, NULL);
> +
> +	return -EINPROGRESS;
> +}
> +
> +static void gemalto_smso_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct gemalto_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("");
> +
> +	g_at_chat_unref(data->mdm);
> +	data->mdm = NULL;
> +	g_at_chat_unref(data->app);
> +	data->app = NULL;
> +
> +	if (ok)
> +		ofono_modem_set_powered(modem, FALSE);

and if not ok?

>  }
>
>  static int gemalto_disable(struct ofono_modem *modem)
> @@ -142,11 +212,14 @@ static int gemalto_disable(struct ofono_modem *modem)
>
>  	DBG("%p", modem);
>
> -	g_at_chat_send(data->app, "AT^SMSO", NULL, NULL, NULL, NULL);
> +	g_at_chat_cancel_all(data->app);
> +	g_at_chat_unregister_all(data->app);
>
> -	ofono_modem_set_data(modem, NULL);
> +	/* Shutdown the modem */
> +	g_at_chat_send(data->app, "AT^SMSO", none_prefix, gemalto_smso_cb,
> +			modem, NULL);

what does this do?  Also, shouldn't you be issuing some sort of CFUN as 
well here?  There's no guarantee that .set_online will be called first.

>
> -	return 0;
> +	return -EINPROGRESS;
>  }
>
>  static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
> @@ -156,11 +229,10 @@ static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
>  	struct ofono_error error;
>
>  	decode_at_error(&error, g_at_result_final_response(result));
> -

See comments about style fixes

>  	cb(&error, cbd->data);
>  }
>
> -static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
> +static void gemalto_set_online(struct ofono_modem *modem, gboolean online,

no, the signature should be ofono_bool_t.  There are no glib types used 
by the oFono driver API (e.g. stuff in include/*)

>  		ofono_modem_online_cb_t cb, void *user_data)
>  {
>  	struct gemalto_data *data = ofono_modem_get_data(modem);
> @@ -173,7 +245,6 @@ static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online,
>  		return;
>
>  	CALLBACK_WITH_FAILURE(cb, cbd->data);
> -

See comments about style fixes

>  	g_free(cbd);
>  }
>
> @@ -185,10 +256,10 @@ static void gemalto_pre_sim(struct ofono_modem *modem)
>  	DBG("%p", modem);
>
>  	ofono_devinfo_create(modem, 0, "atmodem", data->app);
> -	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
>  	ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
> +	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
>
> -	if (sim)
> +	if (sim && data->have_sim == TRUE)
>  		ofono_sim_inserted_notify(sim, TRUE);
>  }
>
>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection
  2017-04-13 17:07           ` Denis Kenzior
@ 2017-04-18  8:38             ` Vincent CESSON
  2017-04-18 15:09               ` Denis Kenzior
  0 siblings, 1 reply; 15+ messages in thread
From: Vincent CESSON @ 2017-04-18  8:38 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 7657 bytes --]

Hi Denis,

Le 2017-04-13 19:07, Denis Kenzior a écrit :
> Hi Vincent,
> 
> On 04/13/2017 03:05 AM, Vincent Cesson wrote:
>> Populate gemalto_data structure.
>> Add sim_state callbacks.
>> Fix enable/disable return value.
>> ---
>>  plugins/gemalto.c | 89 
>> +++++++++++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 80 insertions(+), 9 deletions(-)
>> 
>> diff --git a/plugins/gemalto.c b/plugins/gemalto.c
>> index ab0da8f..5473d2f 100644
>> --- a/plugins/gemalto.c
>> +++ b/plugins/gemalto.c
>> @@ -46,10 +46,14 @@
>>  #include <drivers/atmodem/atutil.h>
>>  #include <drivers/atmodem/vendor.h>
>> 
>> +static const char *none_prefix[] = { NULL };
>> 
>>  struct gemalto_data {
>>  	GAtChat *app;
>>  	GAtChat *mdm;
>> +	struct ofono_sim *sim;
>> +	gboolean have_sim;
>> +	struct at_util_sim_state_query *sim_state_query;
>>  };
>> 
>>  static int gemalto_probe(struct ofono_modem *modem)
>> @@ -57,9 +61,12 @@ static int gemalto_probe(struct ofono_modem 
>> *modem)
>>  	struct gemalto_data *data;
>> 
>>  	data = g_try_new0(struct gemalto_data, 1);
>> +
> 
> Strictly speaking you don't need an empty line here.
> doc/coding-style.txt item M1 mentions an exception at the end of the
> relevant section text.
> 
> I don't care what style variation you use, but please don't mix style
> fixes with code additions in the same patch.
> 
>>  	if (data == NULL)
>>  		return -ENOMEM;
>> 
>> +	data->sim = NULL;
>> +
> 
> This isn't needed, g_try_new0 already memsets the allocated structure 
> to 0.
> 
>>  	ofono_modem_set_data(modem, data);
>> 
>>  	return 0;
>> @@ -69,6 +76,8 @@ static void gemalto_remove(struct ofono_modem 
>> *modem)
>>  {
>>  	struct gemalto_data *data = ofono_modem_get_data(modem);
>> 
>> +	/* Cleanup potential SIM state polling */
>> +	at_util_sim_state_query_free(data->sim_state_query);
>>  	ofono_modem_set_data(modem, NULL);
>>  	g_free(data);
>>  }
>> @@ -89,6 +98,7 @@ static GAtChat *open_device(const char *device)
>>  	DBG("Opening device %s", device);
>> 
>>  	channel = g_at_tty_open(device, NULL);
>> +
> 
> Again, style fix and not relevant to this commit
> 
>>  	if (channel == NULL)
>>  		return NULL;
>> 
>> @@ -103,6 +113,39 @@ static GAtChat *open_device(const char *device)
>>  	return chat;
>>  }
>> 
>> +static void sim_state_cb(gboolean present, gpointer user_data)
>> +{
>> +	struct ofono_modem *modem = user_data;
>> +	struct gemalto_data *data = ofono_modem_get_data(modem);
>> +
>> +	at_util_sim_state_query_free(data->sim_state_query);
>> +	data->sim_state_query = NULL;
>> +
>> +	data->have_sim = present;
>> +	ofono_modem_set_powered(modem, TRUE);
>> +}
>> +
>> +static void cfun_enable(gboolean ok, GAtResult *result, gpointer 
>> user_data)
>> +{
>> +	struct ofono_modem *modem = user_data;
>> +	struct gemalto_data *data = ofono_modem_get_data(modem);
>> +
>> +	if (!ok) {
>> +		g_at_chat_unref(data->app);
>> +		data->app = NULL;
>> +
>> +		g_at_chat_unref(data->mdm);
>> +		data->mdm = NULL;
>> +
>> +		ofono_modem_set_powered(modem, FALSE);
>> +		return;
>> +	}
>> +
>> +	data->sim_state_query = at_util_sim_state_query_new(data->app,
>> +						2, 20, sim_state_cb, modem,
>> +						NULL);
>> +}
>> +
>>  static int gemalto_enable(struct ofono_modem *modem)
>>  {
>>  	struct gemalto_data *data = ofono_modem_get_data(modem);
>> @@ -118,10 +161,12 @@ static int gemalto_enable(struct ofono_modem 
>> *modem)
>> 
>>  	/* Open devices */
>>  	data->app = open_device(app);
>> +
> 
> See comments about style fixes
> 
>>  	if (data->app == NULL)
>>  		return -EINVAL;
>> 
>>  	data->mdm = open_device(mdm);
>> +
> 
> See comments about style fixes
> 
>>  	if (data->mdm == NULL) {
>>  		g_at_chat_unref(data->app);
>>  		data->app = NULL;
>> @@ -133,7 +178,32 @@ static int gemalto_enable(struct ofono_modem 
>> *modem)
>>  		g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
>>  	}
>> 
>> -	return 0;
>> +	g_at_chat_send(data->mdm, "ATE0", none_prefix, NULL, NULL, NULL);
>> +	g_at_chat_send(data->app, "ATE0 +CMEE=1", none_prefix,
>> +			NULL, NULL, NULL);
>> +	g_at_chat_send(data->mdm, "AT&C0", none_prefix, NULL, NULL, NULL);
>> +	g_at_chat_send(data->app, "AT&C0", none_prefix, NULL, NULL, NULL);
>> +
>> +	g_at_chat_send(data->app, "AT+CFUN=4", none_prefix,
>> +			cfun_enable, modem, NULL);
>> +
>> +	return -EINPROGRESS;
>> +}
>> +
>> +static void gemalto_smso_cb(gboolean ok, GAtResult *result, gpointer 
>> user_data)
>> +{
>> +	struct ofono_modem *modem = user_data;
>> +	struct gemalto_data *data = ofono_modem_get_data(modem);
>> +
>> +	DBG("");
>> +
>> +	g_at_chat_unref(data->mdm);
>> +	data->mdm = NULL;
>> +	g_at_chat_unref(data->app);
>> +	data->app = NULL;
>> +
>> +	if (ok)
>> +		ofono_modem_set_powered(modem, FALSE);
> 
> and if not ok?

I mimic the cfun_disable callback of other plugins. See comments about 
AT^SMSO below.

> 
>>  }
>> 
>>  static int gemalto_disable(struct ofono_modem *modem)
>> @@ -142,11 +212,14 @@ static int gemalto_disable(struct ofono_modem 
>> *modem)
>> 
>>  	DBG("%p", modem);
>> 
>> -	g_at_chat_send(data->app, "AT^SMSO", NULL, NULL, NULL, NULL);
>> +	g_at_chat_cancel_all(data->app);
>> +	g_at_chat_unregister_all(data->app);
>> 
>> -	ofono_modem_set_data(modem, NULL);
>> +	/* Shutdown the modem */
>> +	g_at_chat_send(data->app, "AT^SMSO", none_prefix, gemalto_smso_cb,
>> +			modem, NULL);
> 
> what does this do?  Also, shouldn't you be issuing some sort of CFUN
> as well here?  There's no guarantee that .set_online will be called
> first.

AT^SMSO shuts down the modem. I want a real power off, not just 
offline.
Is ".disable" the good place to handle power state? I noticed other 
plugins
just send CFUN=X in ".disable", but in this case how would you handle 
the
"SMSO" command to really power off the modem?

> 
>> 
>> -	return 0;
>> +	return -EINPROGRESS;
>>  }
>> 
>>  static void set_online_cb(gboolean ok, GAtResult *result, gpointer 
>> user_data)
>> @@ -156,11 +229,10 @@ static void set_online_cb(gboolean ok, 
>> GAtResult *result, gpointer user_data)
>>  	struct ofono_error error;
>> 
>>  	decode_at_error(&error, g_at_result_final_response(result));
>> -
> 
> See comments about style fixes
> 
>>  	cb(&error, cbd->data);
>>  }
>> 
>> -static void gemalto_set_online(struct ofono_modem *modem, 
>> ofono_bool_t online,
>> +static void gemalto_set_online(struct ofono_modem *modem, gboolean 
>> online,
> 
> no, the signature should be ofono_bool_t.  There are no glib types
> used by the oFono driver API (e.g. stuff in include/*)
> 
>>  		ofono_modem_online_cb_t cb, void *user_data)
>>  {
>>  	struct gemalto_data *data = ofono_modem_get_data(modem);
>> @@ -173,7 +245,6 @@ static void gemalto_set_online(struct ofono_modem 
>> *modem, ofono_bool_t online,
>>  		return;
>> 
>>  	CALLBACK_WITH_FAILURE(cb, cbd->data);
>> -
> 
> See comments about style fixes
> 
>>  	g_free(cbd);
>>  }
>> 
>> @@ -185,10 +256,10 @@ static void gemalto_pre_sim(struct ofono_modem 
>> *modem)
>>  	DBG("%p", modem);
>> 
>>  	ofono_devinfo_create(modem, 0, "atmodem", data->app);
>> -	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
>>  	ofono_location_reporting_create(modem, 0, "gemaltomodem", 
>> data->app);
>> +	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
>> 
>> -	if (sim)
>> +	if (sim && data->have_sim == TRUE)
>>  		ofono_sim_inserted_notify(sim, TRUE);
>>  }
>> 
>> 
> 
> Regards,
> -Denis

Regards,
-Vincent

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection
  2017-04-18  8:38             ` Vincent CESSON
@ 2017-04-18 15:09               ` Denis Kenzior
  2017-04-18 15:40                 ` [PATCH v4] " Vincent Cesson
  0 siblings, 1 reply; 15+ messages in thread
From: Denis Kenzior @ 2017-04-18 15:09 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]

Hi Vincent,

>> what does this do?  Also, shouldn't you be issuing some sort of CFUN
>> as well here?  There's no guarantee that .set_online will be called
>> first.
>
> AT^SMSO shuts down the modem. I want a real power off, not just offline.
> Is ".disable" the good place to handle power state? I noticed other plugins
> just send CFUN=X in ".disable", but in this case how would you handle the
> "SMSO" command to really power off the modem?
>

For some reason I asssumed SMSO had something to do with SMS :)

If SMSO shuts down the device, then this is fine to put in .disable. 
I've never seen any device use a custom command for this, it has 
generally been CFUN=0.  You can disregard my earlier comment.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4] gemalto: Use sim_state_query for sim detection
  2017-04-18 15:09               ` Denis Kenzior
@ 2017-04-18 15:40                 ` Vincent Cesson
  2017-04-20 17:43                   ` Denis Kenzior
  0 siblings, 1 reply; 15+ messages in thread
From: Vincent Cesson @ 2017-04-18 15:40 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 4159 bytes --]

Populate gemalto_data structure.
Add sim_state callbacks.
Fix enable/disable return value.
---
Change in v4:
 - Remove style fixes (don't mix with code addition)

 plugins/gemalto.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 73 insertions(+), 6 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index ab0da8f..abc20ca 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -46,10 +46,14 @@
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
+static const char *none_prefix[] = { NULL };
 
 struct gemalto_data {
 	GAtChat *app;
 	GAtChat *mdm;
+	struct ofono_sim *sim;
+	gboolean have_sim;
+	struct at_util_sim_state_query *sim_state_query;
 };
 
 static int gemalto_probe(struct ofono_modem *modem)
@@ -69,6 +73,8 @@ static void gemalto_remove(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
 
+	/* Cleanup potential SIM state polling */
+	at_util_sim_state_query_free(data->sim_state_query);
 	ofono_modem_set_data(modem, NULL);
 	g_free(data);
 }
@@ -103,6 +109,39 @@ static GAtChat *open_device(const char *device)
 	return chat;
 }
 
+static void sim_state_cb(gboolean present, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	at_util_sim_state_query_free(data->sim_state_query);
+	data->sim_state_query = NULL;
+
+	data->have_sim = present;
+	ofono_modem_set_powered(modem, TRUE);
+}
+
+static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	if (!ok) {
+		g_at_chat_unref(data->app);
+		data->app = NULL;
+
+		g_at_chat_unref(data->mdm);
+		data->mdm = NULL;
+
+		ofono_modem_set_powered(modem, FALSE);
+		return;
+	}
+
+	data->sim_state_query = at_util_sim_state_query_new(data->app,
+						2, 20, sim_state_cb, modem,
+						NULL);
+}
+
 static int gemalto_enable(struct ofono_modem *modem)
 {
 	struct gemalto_data *data = ofono_modem_get_data(modem);
@@ -133,7 +172,32 @@ static int gemalto_enable(struct ofono_modem *modem)
 		g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
 	}
 
-	return 0;
+	g_at_chat_send(data->mdm, "ATE0", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(data->app, "ATE0 +CMEE=1", none_prefix,
+			NULL, NULL, NULL);
+	g_at_chat_send(data->mdm, "AT&C0", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(data->app, "AT&C0", none_prefix, NULL, NULL, NULL);
+
+	g_at_chat_send(data->app, "AT+CFUN=4", none_prefix,
+			cfun_enable, modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+static void gemalto_smso_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gemalto_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	g_at_chat_unref(data->mdm);
+	data->mdm = NULL;
+	g_at_chat_unref(data->app);
+	data->app = NULL;
+
+	if (ok)
+		ofono_modem_set_powered(modem, FALSE);
 }
 
 static int gemalto_disable(struct ofono_modem *modem)
@@ -142,11 +206,14 @@ static int gemalto_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	g_at_chat_send(data->app, "AT^SMSO", NULL, NULL, NULL, NULL);
+	g_at_chat_cancel_all(data->app);
+	g_at_chat_unregister_all(data->app);
 
-	ofono_modem_set_data(modem, NULL);
+	/* Shutdown the modem */
+	g_at_chat_send(data->app, "AT^SMSO", none_prefix, gemalto_smso_cb,
+			modem, NULL);
 
-	return 0;
+	return -EINPROGRESS;
 }
 
 static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -185,10 +252,10 @@ static void gemalto_pre_sim(struct ofono_modem *modem)
 	DBG("%p", modem);
 
 	ofono_devinfo_create(modem, 0, "atmodem", data->app);
-	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
 	ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
+	sim = ofono_sim_create(modem, 0, "atmodem", data->app);
 
-	if (sim)
+	if (sim && data->have_sim == TRUE)
 		ofono_sim_inserted_notify(sim, TRUE);
 }
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] gemalto: Use sim_state_query for sim detection
  2017-04-18 15:40                 ` [PATCH v4] " Vincent Cesson
@ 2017-04-20 17:43                   ` Denis Kenzior
  0 siblings, 0 replies; 15+ messages in thread
From: Denis Kenzior @ 2017-04-20 17:43 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

Hi Vincent,

On 04/18/2017 10:40 AM, Vincent Cesson wrote:
> Populate gemalto_data structure.
> Add sim_state callbacks.
> Fix enable/disable return value.
> ---
> Change in v4:
>  - Remove style fixes (don't mix with code addition)
>
>  plugins/gemalto.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 73 insertions(+), 6 deletions(-)
>

Applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-04-20 17:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11  9:04 [PATCH 0/1] Gemalto: Improve init process Vincent Cesson
2017-04-11  9:04 ` [PATCH 1/1] gemalto: " Vincent Cesson
2017-04-11 12:30   ` [PATCH v2 " Vincent Cesson
2017-04-11 17:25     ` Denis Kenzior
2017-04-13  8:05       ` [PATCH v3 0/3] " Vincent Cesson
2017-04-13  8:05         ` [PATCH v3 1/3] gemalto: Clean post init functions Vincent Cesson
2017-04-13 16:56           ` Denis Kenzior
2017-04-13  8:05         ` [PATCH v3 2/3] gemalto: Change offline mode, keep USIM available Vincent Cesson
2017-04-13 16:56           ` Denis Kenzior
2017-04-13  8:05         ` [PATCH v3 3/3] gemalto: Use sim_state_query for sim detection Vincent Cesson
2017-04-13 17:07           ` Denis Kenzior
2017-04-18  8:38             ` Vincent CESSON
2017-04-18 15:09               ` Denis Kenzior
2017-04-18 15:40                 ` [PATCH v4] " Vincent Cesson
2017-04-20 17:43                   ` Denis Kenzior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.