[PATCH v2 0/5] Telit LE910 V1 QMI support

[PATCH v2 0/5] Telit LE910 V1 QMI support

[Lukasz Nowak]

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

From: Lukasz Nowak <lnowak@tycoint.com>

Changes from V1:
- split into multiple smaller commits
- added comments in code, documenting issues with the Telit QMI firmware
- disabled non-working low-power operating modes in additional cases
- use mccmnc string instead of truncating an invalid operator name

Lukasz Nowak (5):
  qmimodem: telit LE910 V1 - fix ESN string
  qmimodem: read ss_info at probe time
  qmimodem: detect utf-8 string as operator name
  gobi: Do not use low-power modes for some modems
  udevng: add Telit LE910 V1 support

 drivers/qmimodem/devinfo.c              |  3 ++-
 drivers/qmimodem/gprs.c                 |  7 ++++++
 drivers/qmimodem/network-registration.c | 19 ++++++++++++---
 plugins/gobi.c                          | 41 ++++++++++++++++++++++++++-------
 plugins/udevng.c                        | 24 +++++++++++++++----
 5 files changed, 78 insertions(+), 16 deletions(-)

-- 
2.7.4

[PATCH v2 1/5] qmimodem: telit LE910 V1 - fix ESN string

[Lukasz Nowak]

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

From: Lukasz Nowak <lnowak@tycoint.com>

Telit QMI modems report "0", rather than a NULL string,
if ESN is not available.
---
 drivers/qmimodem/devinfo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/qmimodem/devinfo.c b/drivers/qmimodem/devinfo.c
index 34aec94..77c6874 100644
--- a/drivers/qmimodem/devinfo.c
+++ b/drivers/qmimodem/devinfo.c
@@ -125,7 +125,8 @@ static void get_ids_cb(struct qmi_result *result, void *user_data)
 	}
 
 	str = qmi_result_get_string(result, QMI_DMS_RESULT_ESN);
-	if (!str) {
+	/* Telit qmi modems return a "0" string when ESN is not available. */
+	if (!str || strcmp(str, "0") == 0) {
 		str = qmi_result_get_string(result, QMI_DMS_RESULT_IMEI);
 		if (!str) {
 			CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
-- 
2.7.4

Re: [PATCH v2 1/5] qmimodem: telit LE910 V1 - fix ESN string

[Denis Kenzior]

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

Hi Lukasz,

On 03/22/2017 01:14 PM, Lukasz Nowak wrote:
> From: Lukasz Nowak <lnowak@tycoint.com>
>
> Telit QMI modems report "0", rather than a NULL string,
> if ESN is not available.
> ---
>  drivers/qmimodem/devinfo.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

I went ahead and applied this patch, however you forgot to include 
string.h for this change.  I added a follow-on commit to fix this.

Regards,
-Denis

[PATCH v2 2/5] qmimodem: read ss_info at probe time

[Lukasz Nowak]

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

From: Lukasz Nowak <lnowak@tycoint.com>

LTE modems (observed with Telit LE910 V1) can power on
already registered to a network. In that case, the SS_INFO
change notification will never arrive, and the gprs driver
will never be marked as attached.

To avoid this situation, read SS_INFO at probe time, and if
registered, mark the gprs driver as attached.
---
 drivers/qmimodem/gprs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
index 5e27b0e..983223e 100644
--- a/drivers/qmimodem/gprs.c
+++ b/drivers/qmimodem/gprs.c
@@ -174,6 +174,13 @@ static void create_nas_cb(struct qmi_service *service, void *user_data)
 
 	data->nas = qmi_service_ref(service);
 
+	/*
+	 * First get the SS info - the modem may already be connected,
+	 * and the state-change notification may never arrive
+	 */
+	qmi_service_send(data->nas, QMI_NAS_GET_SS_INFO, NULL,
+					ss_info_notify, gprs, NULL);
+
 	qmi_service_register(data->nas, QMI_NAS_SS_INFO_IND,
 					ss_info_notify, gprs, NULL);
 
-- 
2.7.4

Re: [PATCH v2 2/5] qmimodem: read ss_info at probe time

[Denis Kenzior]

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

Hi Lukasz,

On 03/22/2017 01:14 PM, Lukasz Nowak wrote:
> From: Lukasz Nowak <lnowak@tycoint.com>
>
> LTE modems (observed with Telit LE910 V1) can power on
> already registered to a network. In that case, the SS_INFO
> change notification will never arrive, and the gprs driver
> will never be marked as attached.
>
> To avoid this situation, read SS_INFO at probe time, and if
> registered, mark the gprs driver as attached.
> ---
>  drivers/qmimodem/gprs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>

Applied, thanks.

Regards,
-Denis

[PATCH v2 3/5] qmimodem: detect utf-8 string as operator name

[Lukasz Nowak]

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

From: Lukasz Nowak <lnowak@tycoint.com>

Telit QMI modems can return non-utf-8 characters in plmn-desc.
Observed with LE910-SVG and Verizon. When that happens, libdbus
will abort ofono.
If non-utf-8 characters are detected, use mccmnc string.
---
 drivers/qmimodem/network-registration.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/qmimodem/network-registration.c b/drivers/qmimodem/network-registration.c
index 7389ca5..a3f9cf9 100644
--- a/drivers/qmimodem/network-registration.c
+++ b/drivers/qmimodem/network-registration.c
@@ -64,7 +64,7 @@ static bool extract_ss_info(struct qmi_result *result, int *status,
 	const struct qmi_nas_serving_system *ss;
 	const struct qmi_nas_current_plmn *plmn;
 	uint8_t i, roaming;
-	uint16_t value16, len;
+	uint16_t value16, len, opname_len;
 	uint32_t value32;
 
 	DBG("");
@@ -100,8 +100,21 @@ static bool extract_ss_info(struct qmi_result *result, int *status,
 						GUINT16_FROM_LE(plmn->mcc));
 		snprintf(operator->mnc, OFONO_MAX_MNC_LENGTH + 1, "%02d",
 						GUINT16_FROM_LE(plmn->mnc));
-		strncpy(operator->name, plmn->desc, plmn->desc_len);
-		operator->name[plmn->desc_len] = '\0';
+		opname_len = plmn->desc_len;
+		if (opname_len > OFONO_MAX_OPERATOR_NAME_LENGTH)
+			opname_len = OFONO_MAX_OPERATOR_NAME_LENGTH;
+
+		/*
+		 * Telit QMI modems can return non-utf-8 characters in
+		 * plmn-desc. When that happens, libdbus will abort ofono.
+		 * If non-utf-8 characters are detected, use mccmnc string.
+		 */
+		if (g_utf8_validate(plmn->desc, opname_len, NULL)) {
+			strncpy(operator->name, plmn->desc, opname_len);
+			operator->name[opname_len] = '\0';
+		} else
+			snprintf(operator->name, OFONO_MAX_OPERATOR_NAME_LENGTH,
+					"%s%s",	operator->mcc, operator->mnc);
 
 		DBG("%s (%s:%s)", operator->name, operator->mcc, operator->mnc);
 	}
-- 
2.7.4

Re: [PATCH v2 3/5] qmimodem: detect utf-8 string as operator name

[Denis Kenzior]

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

Hi Lukasz,

On 03/22/2017 01:14 PM, Lukasz Nowak wrote:
> From: Lukasz Nowak <lnowak@tycoint.com>
>
> Telit QMI modems can return non-utf-8 characters in plmn-desc.
> Observed with LE910-SVG and Verizon. When that happens, libdbus
> will abort ofono.
> If non-utf-8 characters are detected, use mccmnc string.
> ---
>  drivers/qmimodem/network-registration.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>

Applied, thanks.

Regards,
-Denis

[PATCH v2 4/5] gobi: Do not use low-power modes for some modems

[Lukasz Nowak]

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

From: Lukasz Nowak <lnowak@tycoint.com>

Telit QMI modems have a problem with the low-power operating modes.
After entering and leaving such a state, UIM service does not return.
The sim card is still marked as powered-down. The QMI interface does
not have a way to power it back on.

To avoid this, keep modems with the "AlwaysOnline" flag online
in the disable-modem and offline-modem procedures.
---
 plugins/gobi.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index 6a78941..d944b39 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -168,7 +168,14 @@ static void get_oper_mode_cb(struct qmi_result *result, void *user_data)
 
 	data->oper_mode = mode;
 
-	switch (data->oper_mode) {
+	/*
+	 * Telit QMI modem must remain online. If powered down, it also
+	 * powers down the sim card, and QMI interface has no way to bring
+	 * it back alive.
+	 */
+	if (ofono_modem_get_boolean(modem, "AlwaysOnline"))
+		ofono_modem_set_powered(modem, TRUE);
+	else switch (data->oper_mode) {
 	case QMI_DMS_OPER_MODE_ONLINE:
 		param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
 					QMI_DMS_OPER_MODE_PERSIST_LOW_POWER);
@@ -353,14 +360,21 @@ static int gobi_disable(struct ofono_modem *modem)
 	qmi_service_cancel_all(data->dms);
 	qmi_service_unregister_all(data->dms);
 
-	param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
-					QMI_DMS_OPER_MODE_PERSIST_LOW_POWER);
-	if (!param)
-		return -ENOMEM;
+	/*
+	 * Telit QMI modem must remain online. If powered down, it also
+	 * powers down the sim card, and QMI interface has no way to bring
+	 * it back alive.
+	 */
+	if (!ofono_modem_get_boolean(modem, "AlwaysOnline")) {
+		param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
+						QMI_DMS_OPER_MODE_PERSIST_LOW_POWER);
+		if (!param)
+			return -ENOMEM;
 
-	if (qmi_service_send(data->dms, QMI_DMS_SET_OPER_MODE, param,
-					power_disable_cb, modem, NULL) > 0)
-		return -EINPROGRESS;
+		if (qmi_service_send(data->dms, QMI_DMS_SET_OPER_MODE, param,
+						power_disable_cb, modem, NULL) > 0)
+			return -EINPROGRESS;
+	}
 
 	shutdown_device(modem);
 
@@ -390,6 +404,17 @@ static void gobi_set_online(struct ofono_modem *modem, ofono_bool_t online,
 
 	DBG("%p %s", modem, online ? "online" : "offline");
 
+	/*
+	 * Telit QMI modem must remain online. If powered down, it also
+	 * powers down the sim card, and QMI interface has no way to bring
+	 * it back alive.
+	 */
+	if (ofono_modem_get_boolean(modem, "AlwaysOnline")) {
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		g_free(cbd);
+		return;
+	}
+
 	if (online)
 		mode = QMI_DMS_OPER_MODE_ONLINE;
 	else
-- 
2.7.4

Re: [PATCH v2 4/5] gobi: Do not use low-power modes for some modems

[Denis Kenzior]

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

Hi Lukasz,

On 03/22/2017 01:14 PM, Lukasz Nowak wrote:
> From: Lukasz Nowak <lnowak@tycoint.com>
>
> Telit QMI modems have a problem with the low-power operating modes.
> After entering and leaving such a state, UIM service does not return.
> The sim card is still marked as powered-down. The QMI interface does
> not have a way to power it back on.
>
> To avoid this, keep modems with the "AlwaysOnline" flag online
> in the disable-modem and offline-modem procedures.
> ---
>  plugins/gobi.c | 41 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/plugins/gobi.c b/plugins/gobi.c
> index 6a78941..d944b39 100644
> --- a/plugins/gobi.c
> +++ b/plugins/gobi.c
> @@ -168,7 +168,14 @@ static void get_oper_mode_cb(struct qmi_result *result, void *user_data)
>
>  	data->oper_mode = mode;
>
> -	switch (data->oper_mode) {
> +	/*
> +	 * Telit QMI modem must remain online. If powered down, it also
> +	 * powers down the sim card, and QMI interface has no way to bring
> +	 * it back alive.
> +	 */
> +	if (ofono_modem_get_boolean(modem, "AlwaysOnline"))
> +		ofono_modem_set_powered(modem, TRUE);
> +	else switch (data->oper_mode) {

else switch is not really our style.  It might be better to simply 
return if AlwaysOnline is true. e.g.

if (ofono_modem_get_boolean(modem, "AlwaysOnline")) {
	ofono_modem_set_powered(...);
	return;
}

switch(...)

>  	case QMI_DMS_OPER_MODE_ONLINE:
>  		param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
>  					QMI_DMS_OPER_MODE_PERSIST_LOW_POWER);
> @@ -353,14 +360,21 @@ static int gobi_disable(struct ofono_modem *modem)
>  	qmi_service_cancel_all(data->dms);
>  	qmi_service_unregister_all(data->dms);
>
> -	param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
> -					QMI_DMS_OPER_MODE_PERSIST_LOW_POWER);
> -	if (!param)
> -		return -ENOMEM;
> +	/*
> +	 * Telit QMI modem must remain online. If powered down, it also
> +	 * powers down the sim card, and QMI interface has no way to bring
> +	 * it back alive.
> +	 */
> +	if (!ofono_modem_get_boolean(modem, "AlwaysOnline")) {
> +		param = qmi_param_new_uint8(QMI_DMS_PARAM_OPER_MODE,
> +						QMI_DMS_OPER_MODE_PERSIST_LOW_POWER);

Please don't go over 80 characters / line

It might be cleaner here to simply jump straight to shutdown_device if 
AlwaysOnline is set.

> +		if (!param)
> +			return -ENOMEM;
>
> -	if (qmi_service_send(data->dms, QMI_DMS_SET_OPER_MODE, param,
> -					power_disable_cb, modem, NULL) > 0)
> -		return -EINPROGRESS;
> +		if (qmi_service_send(data->dms, QMI_DMS_SET_OPER_MODE, param,
> +						power_disable_cb, modem, NULL) > 0)
> +			return -EINPROGRESS;
> +	}
>
>  	shutdown_device(modem);
>
> @@ -390,6 +404,17 @@ static void gobi_set_online(struct ofono_modem *modem, ofono_bool_t online,
>
>  	DBG("%p %s", modem, online ? "online" : "offline");
>
> +	/*
> +	 * Telit QMI modem must remain online. If powered down, it also
> +	 * powers down the sim card, and QMI interface has no way to bring
> +	 * it back alive.
> +	 */
> +	if (ofono_modem_get_boolean(modem, "AlwaysOnline")) {
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);
> +		g_free(cbd);
> +		return;
> +	}
> +

The effect here is that the modem object will never show Modem.Online = 
True.  So applications will think the modem is in Flight mode, e.g. 
radio circuits off.  I don't think this is what you want.

oFono does support modems which have no concept of offline mode, but for 
that a new driver must be implemented with .set_online method being 
NULL.  In this case oFono immediately goes into the post_online state 
after the post_sim state.

Maybe the right way to handle all this is to implement a 
"gobi-always-online" driver instead.  It can live inside gobi.c and 
share most of the function calls.  But the ofono_modem_driver definition 
would modify / implement its own versions of .disable, .enable and leave 
.set_online NULL.

>  	if (online)
>  		mode = QMI_DMS_OPER_MODE_ONLINE;
>  	else
>

Regards,
-Denis

[PATCH v2 5/5] udevng: add Telit LE910 V1 support

[Lukasz Nowak]

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

From: Lukasz Nowak <lnowak@tycoint.com>

Tested with LE910-SVG and Verizon.
---
 plugins/udevng.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 2279bbe..3b15064 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -182,14 +182,15 @@ static gboolean setup_gobi(struct modem_info *modem)
 	const char *qmi = NULL, *mdm = NULL, *net = NULL;
 	const char *gps = NULL, *diag = NULL;
 	GSList *list;
+	gboolean telit = FALSE;
 
 	DBG("%s", modem->syspath);
 
 	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->subsystem);
 
 		if (g_strcmp0(info->interface, "255/255/255") == 0) {
 			if (info->number == NULL)
@@ -199,10 +200,18 @@ static gboolean setup_gobi(struct modem_info *modem)
 			else if (g_strcmp0(info->number, "01") == 0)
 				diag = info->devnode;
 			else if (g_strcmp0(info->number, "02") == 0)
-				mdm = info->devnode;
+				if (g_strcmp0(info->subsystem, "net") == 0)
+					net = info->devnode;
+				else if (g_strcmp0(info->subsystem, "usbmisc") == 0) {
+					telit = TRUE;
+					qmi = info->devnode;
+				} else
+					mdm = info->devnode;
 			else if (g_strcmp0(info->number, "03") == 0)
 				gps = info->devnode;
-		}
+		} else if (g_strcmp0(info->interface, "255/0/0") == 0)
+			if (g_strcmp0(info->number, "04") == 0)
+				mdm = info->devnode;
 	}
 
 	if (qmi == NULL || mdm == NULL || net == NULL)
@@ -215,6 +224,12 @@ static gboolean setup_gobi(struct modem_info *modem)
 	ofono_modem_set_string(modem->modem, "Diag", diag);
 	ofono_modem_set_string(modem->modem, "NetworkInterface", net);
 
+	if (telit) {
+		/* Telit LE910 V1 modems */
+		ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
+		ofono_modem_set_boolean(modem->modem, "AlwaysOnline", TRUE);
+	}
+
 	return TRUE;
 }
 
@@ -1168,6 +1183,7 @@ static struct {
 	{ "mbm",	"cdc_ether",	"0930"		},
 	{ "mbm",	"cdc_ncm",	"0930"		},
 	{ "hso",	"hso"				},
+	{ "gobi",	"option",	"1bc7", "1201"	},
 	{ "gobi",	"qmi_wwan"			},
 	{ "gobi",	"qcserial"			},
 	{ "sierra",	"qmi_wwan",	"1199"		},
-- 
2.7.4