[PATCH 0/3] Telit plugin rework for HE910

[PATCH 0/3] Telit plugin rework for HE910

[Jonas Bonn]

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

This series provides functional support for the Telit HE910 modem
by reworking the initialization sequence.  Though this series has
been tested only on an HE910 device, it seems that this control flow
should work fine for older devices as well given that they share the
same documentation and, presumably, use essentially the same firmware.

Best regards,
Jonas Bonn
South Pole AB

Christopher Vogl (1):
  udevng: support telit modems using cdc-acm driver

Jonas Bonn (2):
  telit: allow plugin to be compiled without BT support
  telit: rework for HE910

 Makefile.am      |   9 ++--
 configure.ac     |   3 ++
 plugins/telit.c  | 156 ++++++++++++++++++++++++++++++++++++++++++++++---------
 plugins/udevng.c |  14 +++--
 4 files changed, 152 insertions(+), 30 deletions(-)

-- 
1.8.1.2

[PATCH 1/3] udevng: support telit modems using cdc-acm driver

[Jonas Bonn]

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

From: Christopher Vogl <christopher.vogl@hale.at>

Add support for Telit modem using the cdc-acm driver, like the Telit
HE910.
---
 plugins/udevng.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index afb02ca..e77e4d9 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -599,8 +599,8 @@ static gboolean setup_telit(struct modem_info *modem)
 	for (list = modem->devices; list; list = list->next) {
 		struct device_info *info = list->data;
 
-		DBG("%s %s %s %s", info->devnode, info->interface,
-						info->number, info->label);
+		DBG("%s %s %s %s %s", info->devnode, info->interface,
+				info->number, info->label, info->sysattr);
 
 		if (g_strcmp0(info->label, "aux") == 0) {
 			aux = info->devnode;
@@ -619,6 +619,13 @@ static gboolean setup_telit(struct modem_info *modem)
 				gps = info->devnode;
 			else if (g_strcmp0(info->number, "03") == 0)
 				aux = info->devnode;
+		} else if (g_strcmp0(info->interface, "2/2/1") == 0) {
+			if (g_strcmp0(info->number, "00") == 0)
+				mdm = info->devnode;
+			else if (g_strcmp0(info->number, "02") == 0)
+				diag = info->devnode;
+			else if (g_strcmp0(info->number, "06") == 0)
+				aux = info->devnode;
 		}
 	}
 
@@ -777,7 +784,7 @@ static struct {
 	{ "alcatel",	setup_alcatel	},
 	{ "novatel",	setup_novatel	},
 	{ "nokia",	setup_nokia	},
-	{ "telit",	setup_telit	},
+	{ "telit",	setup_telit,	"device/interface"	},
 	{ "simcom",	setup_simcom	},
 	{ "zte",	setup_zte	},
 	{ "icera",	setup_icera	},
@@ -987,6 +994,7 @@ static struct {
 	{ "zte",	"option",	"19d2"		},
 	{ "simcom",	"option",	"05c6", "9000"	},
 	{ "telit",	"usbserial",	"1bc7"		},
+	{ "telit",	"cdc_acm",	"1bc7"		},
 	{ "telit",	"option",	"1bc7"		},
 	{ "nokia",	"option",	"0421", "060e"	},
 	{ "nokia",	"option",	"0421", "0623"	},
-- 
1.8.1.2

[PATCH 2/3] telit: allow plugin to be compiled without BT support

[Jonas Bonn]

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

The HE910, for example, does not have Bluetooth, so it may be
desirable to compile the plugin even when Bluetooth is disabled
in the Ofono configuration.
---
 Makefile.am     |  9 ++++++---
 configure.ac    |  3 +++
 plugins/telit.c | 18 ++++++++++++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 0d2ba9f..368836b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -407,13 +407,16 @@ builtin_sources += plugins/samsung.c
 builtin_modules += sim900
 builtin_sources += plugins/sim900.c
 
+builtin_modules += telit
+builtin_sources += plugins/telit.c
+if BLUETOOTH
+builtin_sources += plugins/bluetooth.h
+endif
+
 if BLUETOOTH
 builtin_modules += bluetooth
 builtin_sources += plugins/bluetooth.c plugins/bluetooth.h
 
-builtin_modules += telit
-builtin_sources += plugins/telit.c plugins/bluetooth.h
-
 builtin_modules += sap
 builtin_sources += plugins/sap.c plugins/bluetooth.h
 
diff --git a/configure.ac b/configure.ac
index 450352b..ddd24a4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,6 +182,9 @@ fi
 AC_SUBST(BLUEZ_CFLAGS)
 AC_SUBST(BLUEZ_LIBS)
 AM_CONDITIONAL(BLUETOOTH, test "${enable_bluetooth}" != "no")
+if (test "${enable_bluetooth}" != "no"); then
+	AC_DEFINE(NEED_BLUETOOTH, 1, [Define if bluetooth support is required])
+fi
 
 AC_ARG_WITH([provisiondb], AC_HELP_STRING([--with-provisiondb=FILE],
 	[location of provision database]), [path_provisiondb=${withval}])
diff --git a/plugins/telit.c b/plugins/telit.c
index 79bc421..f661ab9 100644
--- a/plugins/telit.c
+++ b/plugins/telit.c
@@ -58,10 +58,14 @@
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
+#ifdef NEED_BLUETOOTH
 #include "bluetooth.h"
+#endif
 
 static const char *none_prefix[] = { NULL };
+#ifdef NEED_BLUETOOTH
 static const char *rsen_prefix[]= { "#RSEN:", NULL };
+#endif
 
 struct telit_data {
 	GAtChat *chat;		/* AT chat */
@@ -83,6 +87,7 @@ static void telit_debug(const char *str, void *user_data)
 	ofono_info("%s%s", prefix, str);
 }
 
+#ifdef NEED_BLUETOOTH
 static void sap_close_io(struct ofono_modem *modem)
 {
 	struct telit_data *data = ofono_modem_get_data(modem);
@@ -178,6 +183,7 @@ static gboolean hw_event_cb(GIOChannel *hw_io, GIOCondition condition,
 
 	return FALSE;
 }
+#endif
 
 static GAtChat *open_device(struct ofono_modem *modem,
 				const char *key, char *debug)
@@ -285,7 +291,9 @@ static void cfun_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		g_at_chat_unref(data->chat);
 		data->chat = NULL;
 		ofono_modem_set_powered(m, FALSE);
+#ifdef NEED_BLUETOOTH
 		sap_close_io(modem);
+#endif
 		return;
 	}
 
@@ -355,6 +363,7 @@ static int telit_enable(struct ofono_modem *modem)
 	return -EINPROGRESS;
 }
 
+#ifdef NEED_BLUETOOTH
 static void telit_rsen_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -393,6 +402,7 @@ static void rsen_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 }
+#endif
 
 static void cfun_disable_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
@@ -433,6 +443,7 @@ static int telit_disable(struct ofono_modem *modem)
 	return -EINPROGRESS;
 }
 
+#ifdef NEED_BLUETOOTH
 static void rsen_disable_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -542,6 +553,7 @@ static int telit_sap_disable(struct ofono_modem *modem)
 
 	return -EINPROGRESS;
 }
+#endif
 
 static void telit_pre_sim(struct ofono_modem *modem)
 {
@@ -622,6 +634,7 @@ static void telit_post_online(struct ofono_modem *modem)
 		ofono_message_waiting_register(mw);
 }
 
+#ifdef NEED_BLUETOOTH
 static struct bluetooth_sap_driver sap_driver = {
 	.name = "telit",
 	.enable = telit_sap_enable,
@@ -631,6 +644,7 @@ static struct bluetooth_sap_driver sap_driver = {
 	.post_online = telit_post_online,
 	.disable = telit_sap_disable,
 };
+#endif
 
 static int telit_probe(struct ofono_modem *modem)
 {
@@ -644,7 +658,9 @@ static int telit_probe(struct ofono_modem *modem)
 
 	ofono_modem_set_data(modem, data);
 
+#ifdef NEED_BLUETOOTH
 	bluetooth_sap_client_register(&sap_driver, modem);
+#endif
 
 	return 0;
 }
@@ -655,7 +671,9 @@ static void telit_remove(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
+#ifdef NEED_BLUETOOTH
 	bluetooth_sap_client_unregister(modem);
+#endif
 
 	ofono_modem_set_data(modem, NULL);
 
-- 
1.8.1.2

[PATCH 3/3] telit: rework for HE910

[Jonas Bonn]

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

The Telit HE910 turns out to be a rather quirky device and the existing
'telit' driver doesn't really work as is.  This patch is the result of
many hours of trial-and-error and direct consultation with Telit in
order to find an initialization sequence that works, doesn't lock up
the modem, and fits the Ofono paradigm.

1) The SIM status unsolicited messages need to be used in order to know
when the SIM is sufficiently ready to allow certain ther commands to
be able to complete without error.  Unfortunately, these messages are
not reliably issued in a way that allows them to always be caught.  As
such, the modem needs to be prodded to re-issue these messages by:

i)   Using SIMDET=0 to logically 'remove' the SIM before powering on
     the modem
ii)  Setting SIMDET=1 iff a SIM is physically inserted in order to have
     the QSS states re-issued
iii) Finally, setting SIMDET=2 once the modem is powered up to re-enable
     the normal QSS unsolicited messages

2) CFUN state 4 powers off the modem (airplane mode), but it also powers
   off the SIM card.  This doesn't fit well with the Ofono model as the
   GPRS atom won't be created unless the SIM card is usable.  Consultation
   with Telit resulted in the following alternative:

i)   Set CFUN=1 when enabling the modem and set COPS=2 to take the modem
     'offline'
ii)  Set COPS=0 when putting the modem 'online'
iii) CFUN=4 may be used when disabling the modem.  The telit driver
     previously used CFUN=0, but this is actually not an 'offline' state
     at all.

3) The voicecall atom cannot be created until the SIM is ready.

4) The CMER command will always fail

5) The CMER? command will always fail if ofono_sim_inserted_notify is
   called before QSS reaches state 3.

The combination of using CFUN=1 when enabled works around another quirk
in the modem: if the SIM is hotplugged (SIMIN signal toggles) when CFUN=4,
the SIM logic actually wakes up and QSS messages may be issued.  This is
out of sync with the expected behaviour and is difficult to reconcile
with the rest of the control flow in the ofono driver.

The Telit HE910 has a propensity to lock up when echo is enabled.  We
have seen this primarily on the 'modem' port where the modem locks up
after echoing back the command when there is pending command on the
'aux' port.  The changed initialization flow of this patch makes this
problem in general more difficult to hit, but as an additional measure
this patch disables echo on the modem port completely.

These changes have been tested with a Telit HE910 modem only.  As the
documentation for earlier Telit modems is almost identical, I expect
this new control flow to work with those modems as well.  That said, it
would be good if that could be tested with someone with such a modem.
---
 plugins/telit.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 114 insertions(+), 24 deletions(-)

diff --git a/plugins/telit.c b/plugins/telit.c
index f661ab9..2b625da 100644
--- a/plugins/telit.c
+++ b/plugins/telit.c
@@ -63,6 +63,7 @@
 #endif
 
 static const char *none_prefix[] = { NULL };
+static const char *simdet_prefix[]= { "#SIMDET:", NULL };
 #ifdef NEED_BLUETOOTH
 static const char *rsen_prefix[]= { "#RSEN:", NULL };
 #endif
@@ -242,13 +243,32 @@ static void switch_sim_state_status(struct ofono_modem *modem, int status)
 		}
 		break;
 	case 1:	/* SIM inserted */
-	case 2:	/* SIM inserted and PIN unlocked */
+		/* If PIN is required, we need to inform ofono that sim is inserted. */
 		if (data->have_sim == FALSE) {
 			ofono_sim_inserted_notify(data->sim, TRUE);
 			data->have_sim = TRUE;
 		}
 		break;
+	case 2:	/* SIM inserted and PIN unlocked */
+		/* The voicecall atom cannot be created until the SIM is
+		 * unlocked and ready for use.
+		 */
+		ofono_voicecall_create(modem, 0, "atmodem", data->chat);
+
+		/*
+		 * Tell the modem not to automatically initiate auto-attach
+		 * proceedures on its own.  Again, this command requires a
+		 * functional SIM in order to succeed.
+		 */
+		g_at_chat_send(data->chat, "AT#AUTOATT=0", none_prefix,
+				NULL, NULL, NULL);
+
+		break;
 	case 3:	/* SIM inserted, SMS and phonebook ready */
+		if (data->have_sim == FALSE) {
+			ofono_sim_inserted_notify(data->sim, TRUE);
+			data->have_sim = TRUE;
+		}
 		if (data->sms_phonebook_added == FALSE) {
 			ofono_phonebook_create(modem, 0, "atmodem", data->chat);
 			ofono_sms_create(modem, 0, "atmodem", data->chat);
@@ -279,6 +299,46 @@ static void telit_qss_notify(GAtResult *result, gpointer user_data)
 	switch_sim_state_status(modem, status);
 }
 
+static void simdet_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct telit_data *data = ofono_modem_get_data(modem);
+	struct ofono_modem *m = data->sap_modem ? : modem;
+	GAtResultIter iter;
+	int mode;
+	int inserted;
+
+	DBG("%p", modem);
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "#SIMDET:"))
+		return;
+
+	g_at_result_iter_next_number(&iter, &mode);
+	g_at_result_iter_next_number(&iter, &inserted);
+
+	g_at_chat_send(data->chat, "AT#QSS=2", none_prefix, NULL, NULL, NULL);
+
+	/* If the SIM is already physically present, we need to tell the
+	 * modem so by setting SIMDET to 1 in order for it to regenerate
+	 * the QSS messages corresponding to the current state
+	 */
+	if (inserted) {
+		g_at_chat_send(data->chat, "AT#SIMDET=1", none_prefix,
+				NULL, NULL, NULL);
+	}
+
+	/* Restore SIM presence detection via SIMIN pin */
+	g_at_chat_send(data->chat, "AT#SIMDET=2", none_prefix,
+				NULL, NULL, NULL);
+
+	ofono_modem_set_powered(m, TRUE);
+}
+
 static void cfun_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -307,21 +367,14 @@ static void cfun_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	data->have_sim = FALSE;
 	data->sms_phonebook_added = FALSE;
 
-	ofono_modem_set_powered(m, TRUE);
+	/* Set modem 'offline' */
+	g_at_chat_send(data->chat, "AT+COPS=2", none_prefix, NULL, NULL, NULL);
 
-	/*
-	 * Tell the modem not to automatically initiate auto-attach
-	 * proceedures on its own.
+	/* Re-enabling QSS reporting requires that we manually poke the
+	 * modem with its current state... this requires querying SIMDET
 	 */
-	g_at_chat_send(data->chat, "AT#AUTOATT=0", none_prefix,
-				NULL, NULL, NULL);
-
-	/* Follow sim state */
-	g_at_chat_register(data->chat, "#QSS:", telit_qss_notify,
-				FALSE, modem, NULL);
-
-	/* Enable sim state notification */
-	g_at_chat_send(data->chat, "AT#QSS=2", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(data->chat, "AT#SIMDET?", simdet_prefix,
+				simdet_query_cb, modem, NULL);
 }
 
 static int telit_enable(struct ofono_modem *modem)
@@ -349,15 +402,52 @@ static int telit_enable(struct ofono_modem *modem)
 	 */
 	g_at_chat_send(data->chat, "ATE0 +CMEE=1", none_prefix,
 				NULL, NULL, NULL);
+	/* The Telit modem loses command responses when ECHO is on...
+	 * it is a bug and Telit has been informed of it, but the
+	 * easiest way forward for now is to just disable ECHO on
+	 * the modem port.
+	 */
+	g_at_chat_send(data->modem, "ATE0", none_prefix,
+				NULL, NULL, NULL);
 
-	/*
-	 * Disable sim state notification so that we sure get a notification
-	 * when we enable it again later and don't have to query it.
+	/* Telit is quirky, quirky, quirky; getting a reliable QSS
+	 * indication at startup is tricky.
+	 * - QSS transitions aren't necessarily sent when the SIM
+	 *   is first powered on
+	 * - The QSS state, if 1, doesn't automatically transition
+	 *   to 2/3 when QSS mode=2 is set.
+	 * In order to ensure that we get a reliable QSS indication,
+	 * the SIM to needs to be logically (or physically) reinserted
+	 * when the SIM core is powered up.
+	 *
+	 * The SIMIN pin will correctly trigger a QSS transition, but
+	 * it's edge-triggered so that it won't trigger a QSS indication
+	 * unless a SIM is physically reinserted; it's therefore necessary
+	 * to do a "soft" reinsertion by way of the SIMDET command.  The
+	 * sequence that reliably works is:
+	 * - set QSS=0
+	 * - set SIMDET to 0 (SIM logically removed)
+	 * - put modem into CFUN!=4 in order to power up the SIM
+	 * - when modem is powered, set QSS=2
+	 * - check SIMDET value to see if SIM is physically inserted
+	 *   or not; if yes, we need to manually trigger a QSS transition
+	 *   by doing a "soft insertion"; we do this by setting SIMDET=1
+	 *   (SIM logically inserted)
+	 * - then we can set SIMDET=2 to enable SIM hotplug the rest
+	 *   of the way.
 	 */
+
+	/* Follow sim state */
 	g_at_chat_send(data->chat, "AT#QSS=0", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(data->chat, "AT#SIMDET=0", none_prefix,
+				NULL, NULL, NULL);
+	g_at_chat_register(data->chat, "#QSS:", telit_qss_notify,
+				FALSE, modem, NULL);
 
-	/* Set phone functionality */
-	g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
+	/* Set modem to full functionality... this is required to have the
+	 * SIM powered up which is pretty much a prerequisite for Ofono.
+	 */
+	g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
 				cfun_enable_cb, modem, NULL);
 
 	return -EINPROGRESS;
@@ -437,7 +527,7 @@ static int telit_disable(struct ofono_modem *modem)
 	g_at_chat_unregister_all(data->chat);
 
 	/* Power down modem */
-	g_at_chat_send(data->chat, "AT+CFUN=0", none_prefix,
+	g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
 				cfun_disable_cb, modem, NULL);
 
 	return -EINPROGRESS;
@@ -565,9 +655,9 @@ static void telit_pre_sim(struct ofono_modem *modem)
 	DBG("%p", modem);
 
 	ofono_devinfo_create(modem, 0, "atmodem", data->chat);
-	data->sim = ofono_sim_create(modem, OFONO_VENDOR_TELIT, "atmodem",
-					data->chat);
-	ofono_voicecall_create(modem, 0, "atmodem", data->chat);
+	data->sim = ofono_sim_create(modem, OFONO_VENDOR_TELIT,
+			"atmodem", data->chat);
+
 }
 
 static void telit_post_sim(struct ofono_modem *modem)
@@ -604,7 +694,7 @@ static void telit_set_online(struct ofono_modem *modem, ofono_bool_t online,
 {
 	struct telit_data *data = ofono_modem_get_data(modem);
 	struct cb_data *cbd = cb_data_new(cb, user_data);
-	char const *command = online ? "AT+CFUN=1,0" : "AT+CFUN=4,0";
+	char const *command = online ? "AT+COPS=0" : "AT+COPS=2";
 
 	DBG("modem %p %s", modem, online ? "online" : "offline");
 
-- 
1.8.1.2

Re: [PATCH 0/3] Telit plugin rework for HE910

[Etienne Mabille]

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

Thank you, I tested this series of patches on the HE910 yesterday and now
it works just fine.

Etienne Mabille



2013/4/4 Jonas Bonn <jonas@southpole.se>

> This series provides functional support for the Telit HE910 modem
> by reworking the initialization sequence.  Though this series has
> been tested only on an HE910 device, it seems that this control flow
> should work fine for older devices as well given that they share the
> same documentation and, presumably, use essentially the same firmware.
>
> Best regards,
> Jonas Bonn
> South Pole AB
>
> Christopher Vogl (1):
>   udevng: support telit modems using cdc-acm driver
>
> Jonas Bonn (2):
>   telit: allow plugin to be compiled without BT support
>   telit: rework for HE910
>
>  Makefile.am      |   9 ++--
>  configure.ac     |   3 ++
>  plugins/telit.c  | 156
> ++++++++++++++++++++++++++++++++++++++++++++++---------
>  plugins/udevng.c |  14 +++--
>  4 files changed, 152 insertions(+), 30 deletions(-)
>
> --
> 1.8.1.2
>
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1532 bytes --]

Re: [PATCH 3/3] telit: rework for HE910

[Denis Kenzior]

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

Hi Jonas,

On 04/04/2013 03:20 AM, Jonas Bonn wrote:
> The Telit HE910 turns out to be a rather quirky device and the existing
> 'telit' driver doesn't really work as is.  This patch is the result of
> many hours of trial-and-error and direct consultation with Telit in
> order to find an initialization sequence that works, doesn't lock up
> the modem, and fits the Ofono paradigm.

Is the HE910 sufficiently different that it might be easier to simply 
write a dedicated driver for it?  One can even have multiple drivers in 
the telit.c file.

E.g. something like:

static struct ofono_modem_driver telit_driver = {
	.name = "telit",
	...
};

static struct ofono_modem_driver telit_he910_driver = {
	...
};

>
> 1) The SIM status unsolicited messages need to be used in order to know
> when the SIM is sufficiently ready to allow certain ther commands to
> be able to complete without error.  Unfortunately, these messages are
> not reliably issued in a way that allows them to always be caught.  As
> such, the modem needs to be prodded to re-issue these messages by:
>
> i)   Using SIMDET=0 to logically 'remove' the SIM before powering on
>       the modem
> ii)  Setting SIMDET=1 iff a SIM is physically inserted in order to have
>       the QSS states re-issued
> iii) Finally, setting SIMDET=2 once the modem is powered up to re-enable
>       the normal QSS unsolicited messages
>
> 2) CFUN state 4 powers off the modem (airplane mode), but it also powers
>     off the SIM card.  This doesn't fit well with the Ofono model as the
>     GPRS atom won't be created unless the SIM card is usable.  Consultation
>     with Telit resulted in the following alternative:
>

It sounds like the concept of Online is not relevant.  Maybe you should 
simply not implement the .set_enable method.  This way the modem 
initialization logic in the core proceeds straight to post_online state...

> i)   Set CFUN=1 when enabling the modem and set COPS=2 to take the modem
>       'offline'
> ii)  Set COPS=0 when putting the modem 'online'
> iii) CFUN=4 may be used when disabling the modem.  The telit driver
>       previously used CFUN=0, but this is actually not an 'offline' state
>       at all.

... which is better than trying to mess with COPS, since this has 
potential side effects on the netreg atom.

>
> 3) The voicecall atom cannot be created until the SIM is ready.
>

Then move it to the post_sim or post_online state.

> 4) The CMER command will always fail
>
> 5) The CMER? command will always fail if ofono_sim_inserted_notify is
>     called before QSS reaches state 3.
>
> The combination of using CFUN=1 when enabled works around another quirk
> in the modem: if the SIM is hotplugged (SIMIN signal toggles) when CFUN=4,
> the SIM logic actually wakes up and QSS messages may be issued.  This is
> out of sync with the expected behaviour and is difficult to reconcile
> with the rest of the control flow in the ofono driver.
>
> The Telit HE910 has a propensity to lock up when echo is enabled.  We
> have seen this primarily on the 'modem' port where the modem locks up
> after echoing back the command when there is pending command on the
> 'aux' port.  The changed initialization flow of this patch makes this
> problem in general more difficult to hit, but as an additional measure
> this patch disables echo on the modem port completely.
>
> These changes have been tested with a Telit HE910 modem only.  As the
> documentation for earlier Telit modems is almost identical, I expect
> this new control flow to work with those modems as well.  That said, it
> would be good if that could be tested with someone with such a modem.
> ---
>   plugins/telit.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 114 insertions(+), 24 deletions(-)
>
> diff --git a/plugins/telit.c b/plugins/telit.c
> index f661ab9..2b625da 100644
> --- a/plugins/telit.c
> +++ b/plugins/telit.c
> @@ -63,6 +63,7 @@
>   #endif
>
>   static const char *none_prefix[] = { NULL };
> +static const char *simdet_prefix[]= { "#SIMDET:", NULL };
>   #ifdef NEED_BLUETOOTH
>   static const char *rsen_prefix[]= { "#RSEN:", NULL };
>   #endif
> @@ -242,13 +243,32 @@ static void switch_sim_state_status(struct ofono_modem *modem, int status)
>   		}
>   		break;
>   	case 1:	/* SIM inserted */
> -	case 2:	/* SIM inserted and PIN unlocked */
> +		/* If PIN is required, we need to inform ofono that sim is inserted. */
>   		if (data->have_sim == FALSE) {
>   			ofono_sim_inserted_notify(data->sim, TRUE);
>   			data->have_sim = TRUE;
>   		}
>   		break;
> +	case 2:	/* SIM inserted and PIN unlocked */
> +		/* The voicecall atom cannot be created until the SIM is
> +		 * unlocked and ready for use.
> +		 */
> +		ofono_voicecall_create(modem, 0, "atmodem", data->chat);
> +

You can simply move this part to post_sim or post_online

> +		/*
> +		 * Tell the modem not to automatically initiate auto-attach
> +		 * proceedures on its own.  Again, this command requires a
> +		 * functional SIM in order to succeed.
> +		 */
> +		g_at_chat_send(data->chat, "AT#AUTOATT=0", none_prefix,
> +				NULL, NULL, NULL);
> +
> +		break;
>   	case 3:	/* SIM inserted, SMS and phonebook ready */
> +		if (data->have_sim == FALSE) {
> +			ofono_sim_inserted_notify(data->sim, TRUE);
> +			data->have_sim = TRUE;
> +		}
>   		if (data->sms_phonebook_added == FALSE) {
>   			ofono_phonebook_create(modem, 0, "atmodem", data->chat);
>   			ofono_sms_create(modem, 0, "atmodem", data->chat);
> @@ -279,6 +299,46 @@ static void telit_qss_notify(GAtResult *result, gpointer user_data)
>   	switch_sim_state_status(modem, status);
>   }
>
> +static void simdet_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct telit_data *data = ofono_modem_get_data(modem);
> +	struct ofono_modem *m = data->sap_modem ? : modem;
> +	GAtResultIter iter;
> +	int mode;
> +	int inserted;
> +
> +	DBG("%p", modem);
> +
> +	if (!ok)
> +		return;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "#SIMDET:"))
> +		return;
> +
> +	g_at_result_iter_next_number(&iter,&mode);
> +	g_at_result_iter_next_number(&iter,&inserted);
> +
> +	g_at_chat_send(data->chat, "AT#QSS=2", none_prefix, NULL, NULL, NULL);
> +
> +	/* If the SIM is already physically present, we need to tell the
> +	 * modem so by setting SIMDET to 1 in order for it to regenerate
> +	 * the QSS messages corresponding to the current state
> +	 */
> +	if (inserted) {
> +		g_at_chat_send(data->chat, "AT#SIMDET=1", none_prefix,
> +				NULL, NULL, NULL);
> +	}
> +
> +	/* Restore SIM presence detection via SIMIN pin */
> +	g_at_chat_send(data->chat, "AT#SIMDET=2", none_prefix,
> +				NULL, NULL, NULL);
> +
> +	ofono_modem_set_powered(m, TRUE);
> +}
> +
>   static void cfun_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   {
>   	struct ofono_modem *modem = user_data;
> @@ -307,21 +367,14 @@ static void cfun_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   	data->have_sim = FALSE;
>   	data->sms_phonebook_added = FALSE;
>
> -	ofono_modem_set_powered(m, TRUE);
> +	/* Set modem 'offline' */
> +	g_at_chat_send(data->chat, "AT+COPS=2", none_prefix, NULL, NULL, NULL);
>
> -	/*
> -	 * Tell the modem not to automatically initiate auto-attach
> -	 * proceedures on its own.
> +	/* Re-enabling QSS reporting requires that we manually poke the
> +	 * modem with its current state... this requires querying SIMDET
>   	 */
> -	g_at_chat_send(data->chat, "AT#AUTOATT=0", none_prefix,
> -				NULL, NULL, NULL);
> -
> -	/* Follow sim state */
> -	g_at_chat_register(data->chat, "#QSS:", telit_qss_notify,
> -				FALSE, modem, NULL);
> -
> -	/* Enable sim state notification */
> -	g_at_chat_send(data->chat, "AT#QSS=2", none_prefix, NULL, NULL, NULL);
> +	g_at_chat_send(data->chat, "AT#SIMDET?", simdet_prefix,
> +				simdet_query_cb, modem, NULL);
>   }
>
>   static int telit_enable(struct ofono_modem *modem)
> @@ -349,15 +402,52 @@ static int telit_enable(struct ofono_modem *modem)
>   	 */
>   	g_at_chat_send(data->chat, "ATE0 +CMEE=1", none_prefix,
>   				NULL, NULL, NULL);
> +	/* The Telit modem loses command responses when ECHO is on...
> +	 * it is a bug and Telit has been informed of it, but the
> +	 * easiest way forward for now is to just disable ECHO on
> +	 * the modem port.
> +	 */
> +	g_at_chat_send(data->modem, "ATE0", none_prefix,
> +				NULL, NULL, NULL);
>
> -	/*
> -	 * Disable sim state notification so that we sure get a notification
> -	 * when we enable it again later and don't have to query it.
> +	/* Telit is quirky, quirky, quirky; getting a reliable QSS
> +	 * indication at startup is tricky.
> +	 * - QSS transitions aren't necessarily sent when the SIM
> +	 *   is first powered on
> +	 * - The QSS state, if 1, doesn't automatically transition
> +	 *   to 2/3 when QSS mode=2 is set.
> +	 * In order to ensure that we get a reliable QSS indication,
> +	 * the SIM to needs to be logically (or physically) reinserted
> +	 * when the SIM core is powered up.
> +	 *
> +	 * The SIMIN pin will correctly trigger a QSS transition, but
> +	 * it's edge-triggered so that it won't trigger a QSS indication
> +	 * unless a SIM is physically reinserted; it's therefore necessary
> +	 * to do a "soft" reinsertion by way of the SIMDET command.  The
> +	 * sequence that reliably works is:
> +	 * - set QSS=0
> +	 * - set SIMDET to 0 (SIM logically removed)
> +	 * - put modem into CFUN!=4 in order to power up the SIM
> +	 * - when modem is powered, set QSS=2
> +	 * - check SIMDET value to see if SIM is physically inserted
> +	 *   or not; if yes, we need to manually trigger a QSS transition
> +	 *   by doing a "soft insertion"; we do this by setting SIMDET=1
> +	 *   (SIM logically inserted)
> +	 * - then we can set SIMDET=2 to enable SIM hotplug the rest
> +	 *   of the way.
>   	 */
> +
> +	/* Follow sim state */
>   	g_at_chat_send(data->chat, "AT#QSS=0", none_prefix, NULL, NULL, NULL);
> +	g_at_chat_send(data->chat, "AT#SIMDET=0", none_prefix,
> +				NULL, NULL, NULL);
> +	g_at_chat_register(data->chat, "#QSS:", telit_qss_notify,
> +				FALSE, modem, NULL);
>
> -	/* Set phone functionality */
> -	g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
> +	/* Set modem to full functionality... this is required to have the
> +	 * SIM powered up which is pretty much a prerequisite for Ofono.
> +	 */
> +	g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
>   				cfun_enable_cb, modem, NULL);
>
>   	return -EINPROGRESS;
> @@ -437,7 +527,7 @@ static int telit_disable(struct ofono_modem *modem)
>   	g_at_chat_unregister_all(data->chat);
>
>   	/* Power down modem */
> -	g_at_chat_send(data->chat, "AT+CFUN=0", none_prefix,
> +	g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
>   				cfun_disable_cb, modem, NULL);
>
>   	return -EINPROGRESS;
> @@ -565,9 +655,9 @@ static void telit_pre_sim(struct ofono_modem *modem)
>   	DBG("%p", modem);
>
>   	ofono_devinfo_create(modem, 0, "atmodem", data->chat);
> -	data->sim = ofono_sim_create(modem, OFONO_VENDOR_TELIT, "atmodem",
> -					data->chat);
> -	ofono_voicecall_create(modem, 0, "atmodem", data->chat);
> +	data->sim = ofono_sim_create(modem, OFONO_VENDOR_TELIT,
> +			"atmodem", data->chat);
> +
>   }
>
>   static void telit_post_sim(struct ofono_modem *modem)
> @@ -604,7 +694,7 @@ static void telit_set_online(struct ofono_modem *modem, ofono_bool_t online,
>   {
>   	struct telit_data *data = ofono_modem_get_data(modem);
>   	struct cb_data *cbd = cb_data_new(cb, user_data);
> -	char const *command = online ? "AT+CFUN=1,0" : "AT+CFUN=4,0";
> +	char const *command = online ? "AT+COPS=0" : "AT+COPS=2";

As mentioned before, I really don't like this due to potential side 
effects with the netreg atom.

>
>   	DBG("modem %p %s", modem, online ? "online" : "offline");
>

Regards,
-Denis