[PATCH qmi lte v4 1/6] qmi: add missing header inclusion

[PATCH qmi lte v4 1/6] qmi: add missing header inclusion

[Jonas Bonn]

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

---
 drivers/qmimodem/nas.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/qmimodem/nas.h b/drivers/qmimodem/nas.h
index b39b425..b4bc012 100644
--- a/drivers/qmimodem/nas.h
+++ b/drivers/qmimodem/nas.h
@@ -19,6 +19,8 @@
  *
  */
 
+#include <stdint.h>
+
 #define QMI_NAS_RESET			0	/* Reset NAS service state variables */
 #define QMI_NAS_ABORT			1	/* Abort previously issued NAS command */
 #define QMI_NAS_EVENT			2	/* Connection state report indication */
-- 
2.9.3

[PATCH qmi lte v4 2/6] ofono: add missing header inclusions

[Jonas Bonn]

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

This is a "leaf" header and doesn't even have header guards, but
it still seems natural that the header should pull in its own declarations
rather than relying on the including source file to ensure that they
are included.
---
 src/common.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/common.h b/src/common.h
index ef4c58d..05f2a85 100644
--- a/src/common.h
+++ b/src/common.h
@@ -19,6 +19,10 @@
  *
  */
 
+#include <glib.h>
+
+#include <ofono/types.h>
+
 /* 27.007 Section 7.3 <AcT> */
 enum access_technology {
 	ACCESS_TECHNOLOGY_GSM =			0,
-- 
2.9.3

[PATCH qmi lte v4 3/6] qmi: move rat_to_tech() into own module

[Jonas Bonn]

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

We want to use this function from multiple atoms so this patch moves
it out to its own module for NAS-related helper functions.
---
 Makefile.am                             |  1 +
 drivers/qmimodem/nas.c                  | 38 +++++++++++++++++++++++++++++++++
 drivers/qmimodem/nas.h                  |  2 ++
 drivers/qmimodem/network-registration.c | 18 ++--------------
 4 files changed, 43 insertions(+), 16 deletions(-)
 create mode 100644 drivers/qmimodem/nas.c

diff --git a/Makefile.am b/Makefile.am
index b232f97..c894496 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -210,6 +210,7 @@ qmi_sources = drivers/qmimodem/qmi.h drivers/qmimodem/qmi.c \
 					drivers/qmimodem/ctl.h \
 					drivers/qmimodem/dms.h \
 					drivers/qmimodem/nas.h \
+					drivers/qmimodem/nas.c \
 					drivers/qmimodem/uim.h \
 					drivers/qmimodem/wms.h \
 					drivers/qmimodem/wds.h \
diff --git a/drivers/qmimodem/nas.c b/drivers/qmimodem/nas.c
new file mode 100644
index 0000000..48d7f11
--- /dev/null
+++ b/drivers/qmimodem/nas.c
@@ -0,0 +1,38 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017  Jonas Bonn. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include "nas.h"
+
+#include "src/common.h"
+
+int qmi_nas_rat_to_tech(uint8_t rat)
+{
+	switch (rat) {
+	case QMI_NAS_NETWORK_RAT_GSM:
+		return ACCESS_TECHNOLOGY_GSM;
+	case QMI_NAS_NETWORK_RAT_UMTS:
+		return ACCESS_TECHNOLOGY_UTRAN;
+	case QMI_NAS_NETWORK_RAT_LTE:
+		return ACCESS_TECHNOLOGY_EUTRAN;
+	}
+
+	return -1;
+}
diff --git a/drivers/qmimodem/nas.h b/drivers/qmimodem/nas.h
index b4bc012..09807f8 100644
--- a/drivers/qmimodem/nas.h
+++ b/drivers/qmimodem/nas.h
@@ -162,3 +162,5 @@ struct qmi_nas_home_network {
 	uint8_t desc_len;
 	char desc[0];
 } __attribute__((__packed__));
+
+int qmi_nas_rat_to_tech(uint8_t rat);
diff --git a/drivers/qmimodem/network-registration.c b/drivers/qmimodem/network-registration.c
index c29a733..e2a5082 100644
--- a/drivers/qmimodem/network-registration.c
+++ b/drivers/qmimodem/network-registration.c
@@ -43,20 +43,6 @@ struct netreg_data {
 	uint8_t current_rat;
 };
 
-static int rat_to_tech(uint8_t rat)
-{
-	switch (rat) {
-	case QMI_NAS_NETWORK_RAT_GSM:
-		return ACCESS_TECHNOLOGY_GSM;
-	case QMI_NAS_NETWORK_RAT_UMTS:
-		return ACCESS_TECHNOLOGY_UTRAN;
-	case QMI_NAS_NETWORK_RAT_LTE:
-		return ACCESS_TECHNOLOGY_EUTRAN;
-	}
-
-	return -1;
-}
-
 static bool extract_ss_info(struct qmi_result *result, int *status,
 				int *lac, int *cellid, int *tech,
 				struct ofono_network_operator *operator)
@@ -82,7 +68,7 @@ static bool extract_ss_info(struct qmi_result *result, int *status,
 	for (i = 0; i < ss->radio_if_count; i++) {
 		DBG("radio in use %d", ss->radio_if[i]);
 
-		*tech = rat_to_tech(ss->radio_if[i]);
+		*tech = qmi_nas_rat_to_tech(ss->radio_if[i]);
 	}
 
 	if (qmi_result_get_uint8(result, QMI_NAS_RESULT_ROAMING_STATUS,
@@ -278,7 +264,7 @@ static void scan_nets_cb(struct qmi_result *result, void *user_data)
 		DBG("%03d:%02d %d", netrat->info[i].mcc, netrat->info[i].mnc,
 							netrat->info[i].rat);
 
-		list[i].tech = rat_to_tech(netrat->info[i].rat);
+		list[i].tech = qmi_nas_rat_to_tech(netrat->info[i].rat);
 	}
 
 done:
-- 
2.9.3

[PATCH qmi lte v4 4/6] qmi: activate default bearer context for LTE networks

[Jonas Bonn]

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

When the modem attaches to an LTE network, a default bearer is
automatically negotiated using the "defalt profile" settings.  The
QMI modem, however, does not given any explicit indication that
the bearer exists; instead, we must assume its existence based on
the network registration state.

This patch extends the GPRS atom to signal the presence of a
default bearer when it detects network connectivity on an LTE
network.

The GPRS atom gets a new 'attached' property based on whether it has
been manually attached or automatically "attached" by virtue of
connecting to an LTE network.  Here we abuse the word "attach"
since attach'ing isn't really an LTE concept, but it fits with
the model that Ofono wants.
---
 drivers/qmimodem/gprs.c | 102 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 83 insertions(+), 19 deletions(-)

diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
index 05ad4bd..c5bdf5e 100644
--- a/drivers/qmimodem/gprs.c
+++ b/drivers/qmimodem/gprs.c
@@ -23,6 +23,8 @@
 #include <config.h>
 #endif
 
+#include <glib.h>	/* this include belongs in common.h */
+
 #include <ofono/log.h>
 #include <ofono/modem.h>
 #include <ofono/gprs.h>
@@ -30,16 +32,20 @@
 #include "qmi.h"
 #include "nas.h"
 
+#include "src/common.h"
 #include "qmimodem.h"
 
 struct gprs_data {
 	struct qmi_service *nas;
+	int attached;
 };
 
-static bool extract_ss_info(struct qmi_result *result, int *status)
+static bool extract_ss_info(struct qmi_result *result, int *status, int *tech)
 {
 	const struct qmi_nas_serving_system *ss;
+	uint8_t roaming;
 	uint16_t len;
+	int i;
 
 	DBG("");
 
@@ -47,25 +53,71 @@ static bool extract_ss_info(struct qmi_result *result, int *status)
 	if (!ss)
 		return false;
 
-	if (ss->ps_state == QMI_NAS_ATTACH_STATE_ATTACHED)
-		*status = 0x01;
-	else
-		*status = 0x00;
+	*status = ss->status;
+
+	*tech = -1;
+	for (i = 0; i < ss->radio_if_count; i++) {
+		DBG("radio in use %d", ss->radio_if[i]);
+
+		*tech = qmi_nas_rat_to_tech(ss->radio_if[i]);
+	}
+
+	if (qmi_result_get_uint8(result, QMI_NAS_RESULT_ROAMING_STATUS,
+								&roaming)) {
+		if (ss->status == 1 && roaming == 0)
+			*status = NETWORK_REGISTRATION_STATUS_ROAMING;
+	}
 
 	return true;
 }
 
-static void ss_info_notify(struct qmi_result *result, void *user_data)
+static int handle_ss_info(struct qmi_result* result, struct ofono_gprs* gprs)
 {
-	struct ofono_gprs *gprs = user_data;
+	struct gprs_data *data = ofono_gprs_get_data(gprs);
 	int status;
+	int tech;
 
 	DBG("");
 
-	if (!extract_ss_info(result, &status))
-		return;
+	if (!extract_ss_info(result, &status, &tech))
+		return -1;
+
+	switch (status) {
+	case NETWORK_REGISTRATION_STATUS_REGISTERED:
+	case NETWORK_REGISTRATION_STATUS_ROAMING:
+		if (tech == ACCESS_TECHNOLOGY_EUTRAN && !data->attached) {
+			/* On LTE we are effectively always attached; and
+			 * the default bearer is established as soon as the
+			 * network is joined.
+			 */
+			data->attached = 1;
+			/* FIXME: query default profile number and APN
+			 * instead of assuming profile 1
+			 */
+			ofono_gprs_cid_activated(gprs, 1, "");
+		}
+		break;
+	default:
+		break;
+	}
 
-	ofono_gprs_status_notify(gprs, status);
+	if (!data->attached)
+		status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
+
+	return status;
+}
+
+static void ss_info_notify(struct qmi_result* result, void *user_data)
+{
+	struct ofono_gprs* gprs = user_data;
+	int status;
+
+	DBG("");
+
+	status = handle_ss_info(result, gprs);
+
+	if (status >= 0)
+		ofono_gprs_status_notify(gprs, status);
 }
 
 static void attach_detach_cb(struct qmi_result *result, void *user_data)
@@ -100,6 +152,8 @@ static void qmi_set_attached(struct ofono_gprs *gprs, int attached,
 
 	DBG("attached %d", attached);
 
+	data->attached = attached;
+
 	if (attached)
 		action = QMI_NAS_ATTACH_ACTION_ATTACH;
 	else
@@ -121,25 +175,29 @@ error:
 	g_free(cbd);
 }
 
-static void get_ss_info_cb(struct qmi_result *result, void *user_data)
+static void get_ss_info_cb(struct qmi_result* result, void *user_data)
 {
 	struct cb_data *cbd = user_data;
+	struct ofono_gprs* gprs = cbd->user;
 	ofono_gprs_status_cb_t cb = cbd->cb;
 	int status;
 
 	DBG("");
 
-	if (qmi_result_set_error(result, NULL)) {
-		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
-		return;
-	}
+	if (qmi_result_set_error(result, NULL))
+		goto error;
 
-	if (!extract_ss_info(result, &status)) {
-		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
-		return;
-	}
+	status = handle_ss_info(result, gprs);
+
+	if (status < 0)
+		goto error;
 
 	CALLBACK_WITH_SUCCESS(cb, status, cbd->data);
+
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
 }
 
 static void qmi_attached_status(struct ofono_gprs *gprs,
@@ -150,6 +208,12 @@ static void qmi_attached_status(struct ofono_gprs *gprs,
 
 	DBG("");
 
+	if (!data->attached) {
+		CALLBACK_WITH_SUCCESS(cb,
+			NETWORK_REGISTRATION_STATUS_NOT_REGISTERED, user_data);
+	}
+
+	cbd->user = gprs;
 	if (qmi_service_send(data->nas, QMI_NAS_GET_SS_INFO, NULL,
 					get_ss_info_cb, cbd, g_free) > 0)
 		return;
-- 
2.9.3

[PATCH qmi lte v4 5/6] qmi: add LTE atom driver

[Jonas Bonn]

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

This patch adds the boilerplate for an LTE atom for QMI modems.

This atom is supposed to set the APN that the default bearer uses
to connect to the network.  The part that is missing currently is
actually writing the APN into the modem's default profile.

In many cases, however, the network does not actually require a
correct APN in order to connect to the network so it's moot whether
or not this atom actually completes its task!  For now, thus, this
atom just emits a warning so that this can be fully implemented
when there's a network that cares about the APN to test against.

For completeness, what is supposed to happen is that this atom
is supposed to call "Modify Profile" on the "default" profile,
setting the APN.  The default profile should probably be queried
because it can be set to something other than profile number one (1).
Once configured, the default profile settings are used when the
modem connects to the network; for this reason, the LTE atom needs
to be instantiated in post_sim, before the modem is set online.
---
 Makefile.am                 |   1 +
 drivers/qmimodem/lte.c      | 143 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/qmimodem/qmimodem.c |   2 +
 drivers/qmimodem/qmimodem.h |   3 +
 4 files changed, 149 insertions(+)
 create mode 100644 drivers/qmimodem/lte.c

diff --git a/Makefile.am b/Makefile.am
index c894496..60c205a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -232,6 +232,7 @@ builtin_sources += $(qmi_sources) \
 			drivers/qmimodem/ussd.c \
 			drivers/qmimodem/gprs.c \
 			drivers/qmimodem/gprs-context.c \
+			drivers/qmimodem/lte.c \
 			drivers/qmimodem/radio-settings.c \
 			drivers/qmimodem/location-reporting.c
 
diff --git a/drivers/qmimodem/lte.c b/drivers/qmimodem/lte.c
new file mode 100644
index 0000000..e131b47
--- /dev/null
+++ b/drivers/qmimodem/lte.c
@@ -0,0 +1,143 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017  Jonas Bonn. All rights reserved.
+ *  Copyright (C) 2017  South Pole Consulting AB. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include <ofono/modem.h>
+#include <ofono/gprs-context.h>
+#include <ofono/log.h>
+#include <ofono/lte.h>
+
+#include "qmi.h"
+#include "wds.h"
+
+#include "qmimodem.h"
+
+struct lte_data {
+	struct qmi_service *wds;
+	char* apn;
+};
+
+static void qmimodem_lte_set_default_attach_info(const struct ofono_lte *lte,
+			const struct ofono_lte_default_attach_info *info,
+			ofono_lte_cb_t cb, void *data)
+{
+	struct lte_data *ldd = ofono_lte_get_data(lte);
+
+	DBG("LTE config with APN: %s", info->apn);
+
+	g_free(ldd->apn);
+
+	if (strlen(info->apn) > 0)
+		ldd->apn = g_strdup(info->apn);
+	else
+		ldd->apn = NULL;
+
+	/*
+	 * FIXME: The above is not sufficient.  The APN needs to be set
+	 * to the 'default' profile so that the modem uses it when
+	 * connecting the default bearer.
+	 */
+	ofono_warn("LTE APN needs to be written to default profile");
+
+	CALLBACK_WITH_SUCCESS(cb, data);
+}
+
+static void create_wds_cb(struct qmi_service *service, void *user_data)
+{
+	struct ofono_lte *lte = user_data;
+	struct lte_data *data = ofono_lte_get_data(lte);
+
+	DBG("");
+
+	if (!service) {
+		ofono_error("Failed to request WDS service");
+		ofono_lte_remove(lte);
+		return;
+	}
+
+	data->wds = qmi_service_ref(service);
+
+	ofono_lte_register(lte);
+}
+
+static int qmimodem_lte_probe(struct ofono_lte *lte, void *data)
+{
+	struct qmi_device *device = data;
+	struct lte_data *ldd;
+
+	DBG("qmimodem lte probe");
+
+	ldd = g_try_new0(struct lte_data, 1);
+	if (!ldd)
+		return -ENOMEM;
+
+	ofono_lte_set_data(lte, ldd);
+
+	qmi_service_create(device, QMI_SERVICE_WDS,
+					create_wds_cb, lte, NULL);
+
+	return 0;
+}
+
+static void qmimodem_lte_remove(struct ofono_lte *lte)
+{
+	struct lte_data *ldd = ofono_lte_get_data(lte);
+
+	DBG("");
+
+	ofono_lte_set_data(lte, NULL);
+
+	qmi_service_unregister_all(ldd->wds);
+
+	qmi_service_unref(ldd->wds);
+
+	g_free(ldd->apn);
+
+	g_free(ldd);
+}
+
+static struct ofono_lte_driver driver = {
+	.name				= "qmimodem",
+	.probe				= qmimodem_lte_probe,
+	.remove				= qmimodem_lte_remove,
+	.set_default_attach_info	= qmimodem_lte_set_default_attach_info,
+};
+
+void qmi_lte_init(void)
+{
+	ofono_lte_driver_register(&driver);
+}
+
+void qmi_lte_exit(void)
+{
+	ofono_lte_driver_unregister(&driver);
+}
diff --git a/drivers/qmimodem/qmimodem.c b/drivers/qmimodem/qmimodem.c
index 959a901..2ce5c02 100644
--- a/drivers/qmimodem/qmimodem.c
+++ b/drivers/qmimodem/qmimodem.c
@@ -39,6 +39,7 @@ static int qmimodem_init(void)
 	qmi_ussd_init();
 	qmi_gprs_init();
 	qmi_gprs_context_init();
+	qmi_lte_init();
 	qmi_radio_settings_init();
 	qmi_location_reporting_init();
 
@@ -49,6 +50,7 @@ static void qmimodem_exit(void)
 {
 	qmi_location_reporting_exit();
 	qmi_radio_settings_exit();
+	qmi_lte_exit();
 	qmi_gprs_context_exit();
 	qmi_gprs_exit();
 	qmi_ussd_exit();
diff --git a/drivers/qmimodem/qmimodem.h b/drivers/qmimodem/qmimodem.h
index 1fc8682..4667334 100644
--- a/drivers/qmimodem/qmimodem.h
+++ b/drivers/qmimodem/qmimodem.h
@@ -48,6 +48,9 @@ extern void qmi_gprs_exit(void);
 extern void qmi_gprs_context_init(void);
 extern void qmi_gprs_context_exit(void);
 
+extern void qmi_lte_init(void);
+extern void qmi_lte_exit(void);
+
 extern void qmi_radio_settings_init(void);
 extern void qmi_radio_settings_exit(void);
 
-- 
2.9.3

[PATCH qmi lte v4 6/6] gobi: add LTE atom

[Jonas Bonn]

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

This atom needs to be created in post_sim so that the APN can be
written to the default profile before the modem attempts to use the
setting to connect to the network.
---
 plugins/gobi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index df35f94..a339740 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -42,6 +42,7 @@
 #include <ofono/ussd.h>
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
+#include <ofono/lte.h>
 #include <ofono/radio-settings.h>
 #include <ofono/location-reporting.h>
 #include <ofono/log.h>
@@ -469,6 +470,8 @@ static void gobi_post_sim(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
+	ofono_lte_create(modem, "qmimodem", data->device);
+
 	if (data->features & GOBI_CAT)
 		ofono_stk_create(modem, 0, "qmimodem", data->device);
 	else if (data->features & GOBI_CAT_OLD)
-- 
2.9.3

Re: [PATCH qmi lte v4 1/6] qmi: add missing header inclusion

[Denis Kenzior]

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

Hi Jonas,

On 04/18/2017 02:50 AM, Jonas Bonn wrote:
> ---
>  drivers/qmimodem/nas.h | 2 ++
>  1 file changed, 2 insertions(+)
>

Patches 1-3 applied, thanks.

Regards,
-Denis

Re: [PATCH qmi lte v4 5/6] qmi: add LTE atom driver

[Denis Kenzior]

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

Hi Jonas,

On 04/18/2017 02:50 AM, Jonas Bonn wrote:
> This patch adds the boilerplate for an LTE atom for QMI modems.
>
> This atom is supposed to set the APN that the default bearer uses
> to connect to the network.  The part that is missing currently is
> actually writing the APN into the modem's default profile.
>
> In many cases, however, the network does not actually require a
> correct APN in order to connect to the network so it's moot whether
> or not this atom actually completes its task!  For now, thus, this
> atom just emits a warning so that this can be fully implemented
> when there's a network that cares about the APN to test against.
>
> For completeness, what is supposed to happen is that this atom
> is supposed to call "Modify Profile" on the "default" profile,
> setting the APN.  The default profile should probably be queried
> because it can be set to something other than profile number one (1).
> Once configured, the default profile settings are used when the
> modem connects to the network; for this reason, the LTE atom needs
> to be instantiated in post_sim, before the modem is set online.
> ---
>  Makefile.am                 |   1 +
>  drivers/qmimodem/lte.c      | 143 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/qmimodem/qmimodem.c |   2 +
>  drivers/qmimodem/qmimodem.h |   3 +
>  4 files changed, 149 insertions(+)
>  create mode 100644 drivers/qmimodem/lte.c
>
> diff --git a/Makefile.am b/Makefile.am
> index c894496..60c205a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -232,6 +232,7 @@ builtin_sources += $(qmi_sources) \
>  			drivers/qmimodem/ussd.c \
>  			drivers/qmimodem/gprs.c \
>  			drivers/qmimodem/gprs-context.c \
> +			drivers/qmimodem/lte.c \
>  			drivers/qmimodem/radio-settings.c \
>  			drivers/qmimodem/location-reporting.c
>
> diff --git a/drivers/qmimodem/lte.c b/drivers/qmimodem/lte.c
> new file mode 100644
> index 0000000..e131b47
> --- /dev/null
> +++ b/drivers/qmimodem/lte.c
> @@ -0,0 +1,143 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2017  Jonas Bonn. All rights reserved.
> + *  Copyright (C) 2017  South Pole Consulting AB. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +
> +#include <ofono/modem.h>
> +#include <ofono/gprs-context.h>
> +#include <ofono/log.h>
> +#include <ofono/lte.h>
> +
> +#include "qmi.h"
> +#include "wds.h"
> +
> +#include "qmimodem.h"
> +
> +struct lte_data {
> +	struct qmi_service *wds;
> +	char* apn;
> +};
> +
> +static void qmimodem_lte_set_default_attach_info(const struct ofono_lte *lte,
> +			const struct ofono_lte_default_attach_info *info,
> +			ofono_lte_cb_t cb, void *data)
> +{
> +	struct lte_data *ldd = ofono_lte_get_data(lte);
> +
> +	DBG("LTE config with APN: %s", info->apn);
> +
> +	g_free(ldd->apn);
> +
> +	if (strlen(info->apn) > 0)
> +		ldd->apn = g_strdup(info->apn);
> +	else
> +		ldd->apn = NULL;
> +

Why do you want to store the APN inside the driver data?

> +	/*
> +	 * FIXME: The above is not sufficient.  The APN needs to be set
> +	 * to the 'default' profile so that the modem uses it when
> +	 * connecting the default bearer.
> +	 */
> +	ofono_warn("LTE APN needs to be written to default profile");
> +
> +	CALLBACK_WITH_SUCCESS(cb, data);
> +}
> +
> +static void create_wds_cb(struct qmi_service *service, void *user_data)
> +{
> +	struct ofono_lte *lte = user_data;
> +	struct lte_data *data = ofono_lte_get_data(lte);
> +
> +	DBG("");
> +
> +	if (!service) {
> +		ofono_error("Failed to request WDS service");
> +		ofono_lte_remove(lte);
> +		return;
> +	}
> +
> +	data->wds = qmi_service_ref(service);
> +
> +	ofono_lte_register(lte);
> +}
> +
> +static int qmimodem_lte_probe(struct ofono_lte *lte, void *data)
> +{
> +	struct qmi_device *device = data;
> +	struct lte_data *ldd;
> +
> +	DBG("qmimodem lte probe");
> +
> +	ldd = g_try_new0(struct lte_data, 1);
> +	if (!ldd)
> +		return -ENOMEM;
> +
> +	ofono_lte_set_data(lte, ldd);
> +
> +	qmi_service_create(device, QMI_SERVICE_WDS,
> +					create_wds_cb, lte, NULL);
> +

So I talked to Marcel about this and it seems a QMI service cannot be 
created multiple times for the same device.  So I think this should fail 
and you should be using qmi_service_create_shared instead.

> +	return 0;
> +}
> +
> +static void qmimodem_lte_remove(struct ofono_lte *lte)
> +{
> +	struct lte_data *ldd = ofono_lte_get_data(lte);
> +
> +	DBG("");
> +
> +	ofono_lte_set_data(lte, NULL);
> +
> +	qmi_service_unregister_all(ldd->wds);
> +
> +	qmi_service_unref(ldd->wds);
> +
> +	g_free(ldd->apn);
> +
> +	g_free(ldd);
> +}
> +

The rest looks fine.

Regards,
-Denis

Re: [PATCH qmi lte v4 4/6] qmi: activate default bearer context for LTE networks

[Denis Kenzior]

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

Hi Jonas,

On 04/18/2017 02:50 AM, Jonas Bonn wrote:
> When the modem attaches to an LTE network, a default bearer is
> automatically negotiated using the "defalt profile" settings.  The
> QMI modem, however, does not given any explicit indication that
> the bearer exists; instead, we must assume its existence based on
> the network registration state.
>
> This patch extends the GPRS atom to signal the presence of a
> default bearer when it detects network connectivity on an LTE
> network.
>
> The GPRS atom gets a new 'attached' property based on whether it has
> been manually attached or automatically "attached" by virtue of
> connecting to an LTE network.  Here we abuse the word "attach"
> since attach'ing isn't really an LTE concept, but it fits with
> the model that Ofono wants.
> ---
>  drivers/qmimodem/gprs.c | 102 +++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 83 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
> index 05ad4bd..c5bdf5e 100644
> --- a/drivers/qmimodem/gprs.c
> +++ b/drivers/qmimodem/gprs.c
> @@ -23,6 +23,8 @@
>  #include <config.h>
>  #endif
>
> +#include <glib.h>	/* this include belongs in common.h */
> +
>  #include <ofono/log.h>
>  #include <ofono/modem.h>
>  #include <ofono/gprs.h>
> @@ -30,16 +32,20 @@
>  #include "qmi.h"
>  #include "nas.h"
>
> +#include "src/common.h"
>  #include "qmimodem.h"
>
>  struct gprs_data {
>  	struct qmi_service *nas;
> +	int attached;
>  };
>
> -static bool extract_ss_info(struct qmi_result *result, int *status)
> +static bool extract_ss_info(struct qmi_result *result, int *status, int *tech)
>  {
>  	const struct qmi_nas_serving_system *ss;
> +	uint8_t roaming;
>  	uint16_t len;
> +	int i;
>
>  	DBG("");
>
> @@ -47,25 +53,71 @@ static bool extract_ss_info(struct qmi_result *result, int *status)
>  	if (!ss)
>  		return false;
>
> -	if (ss->ps_state == QMI_NAS_ATTACH_STATE_ATTACHED)
> -		*status = 0x01;
> -	else
> -		*status = 0x00;

Hmm.  So QMI reports attached_status and network registration status 
separately.  oFono wants the network registration status (which is 
usually voice related for 2G/3G) on the netreg atom and the PS (packet 
service) attach status on the gprs atom.  We follow 27.007 whenever 
possible, so the gprs status can have the same enumeration as the netreg 
status, e.g.:

not registered, registered, roaming, searching, etc.

However, in practice not registered and registered are the only required 
values.  QMI & ISI report PS attached status at on/off.  But one can 
generate the 3GPP +CGREG equivalent knowing the network registration 
status and the PS attach status.  I hope that made sense.

> +	*status = ss->status;

So here, you really want to use the ps_state, at least for 2G/3G.

> +
> +	*tech = -1;
> +	for (i = 0; i < ss->radio_if_count; i++) {
> +		DBG("radio in use %d", ss->radio_if[i]);
> +
> +		*tech = qmi_nas_rat_to_tech(ss->radio_if[i]);
> +	}
> +
> +	if (qmi_result_get_uint8(result, QMI_NAS_RESULT_ROAMING_STATUS,
> +								&roaming)) {
> +		if (ss->status == 1 && roaming == 0)
> +			*status = NETWORK_REGISTRATION_STATUS_ROAMING;

This should really be ss->ps_status == 1 && ss->status && roaming == 0. 
However, as mentioned earlier, reporting roaming status to gprs is not 
strictly necessary.  As long as netreg reports roaming status correctly, 
the RoamingAllowed logic should still work.

> +	}
>
>  	return true;
>  }
>
> -static void ss_info_notify(struct qmi_result *result, void *user_data)
> +static int handle_ss_info(struct qmi_result* result, struct ofono_gprs* gprs)

Can you put the '*' in its original place please?

>  {
> -	struct ofono_gprs *gprs = user_data;
> +	struct gprs_data *data = ofono_gprs_get_data(gprs);
>  	int status;
> +	int tech;
>
>  	DBG("");
>
> -	if (!extract_ss_info(result, &status))
> -		return;
> +	if (!extract_ss_info(result, &status, &tech))
> +		return -1;
> +
> +	switch (status) {
> +	case NETWORK_REGISTRATION_STATUS_REGISTERED:
> +	case NETWORK_REGISTRATION_STATUS_ROAMING:
> +		if (tech == ACCESS_TECHNOLOGY_EUTRAN && !data->attached) {
> +			/* On LTE we are effectively always attached; and
> +			 * the default bearer is established as soon as the
> +			 * network is joined.
> +			 */
> +			data->attached = 1;
> +			/* FIXME: query default profile number and APN
> +			 * instead of assuming profile 1
> +			 */
> +			ofono_gprs_cid_activated(gprs, 1, "");
> +		}
> +		break;
> +	default:
> +		break;
> +	}
>
> -	ofono_gprs_status_notify(gprs, status);
> +	if (!data->attached)
> +		status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;

I don't know what you're doing here?  You really need to tell the core 
what is the actual status straight from the firmware..

> +
> +	return status;
> +}
> +
> +static void ss_info_notify(struct qmi_result* result, void *user_data)
> +{
> +	struct ofono_gprs* gprs = user_data;
> +	int status;
> +
> +	DBG("");
> +
> +	status = handle_ss_info(result, gprs);
> +
> +	if (status >= 0)
> +		ofono_gprs_status_notify(gprs, status);
>  }
>
>  static void attach_detach_cb(struct qmi_result *result, void *user_data)
> @@ -100,6 +152,8 @@ static void qmi_set_attached(struct ofono_gprs *gprs, int attached,
>
>  	DBG("attached %d", attached);
>
> +	data->attached = attached;
> +

So it seems you're using attached to try and trigger the initial attach 
logic.  However, I'm not sure it works if you register to LTE, lose 
network coverage and then come back to LTE coverage?

>  	if (attached)
>  		action = QMI_NAS_ATTACH_ACTION_ATTACH;
>  	else
> @@ -121,25 +175,29 @@ error:
>  	g_free(cbd);
>  }
>
> -static void get_ss_info_cb(struct qmi_result *result, void *user_data)
> +static void get_ss_info_cb(struct qmi_result* result, void *user_data)
>  {
>  	struct cb_data *cbd = user_data;
> +	struct ofono_gprs* gprs = cbd->user;
>  	ofono_gprs_status_cb_t cb = cbd->cb;
>  	int status;
>
>  	DBG("");
>
> -	if (qmi_result_set_error(result, NULL)) {
> -		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> -		return;
> -	}
> +	if (qmi_result_set_error(result, NULL))
> +		goto error;
>
> -	if (!extract_ss_info(result, &status)) {
> -		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
> -		return;
> -	}
> +	status = handle_ss_info(result, gprs);
> +
> +	if (status < 0)
> +		goto error;
>
>  	CALLBACK_WITH_SUCCESS(cb, status, cbd->data);
> +
> +	return;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
>  }
>
>  static void qmi_attached_status(struct ofono_gprs *gprs,
> @@ -150,6 +208,12 @@ static void qmi_attached_status(struct ofono_gprs *gprs,
>
>  	DBG("");
>
> +	if (!data->attached) {
> +		CALLBACK_WITH_SUCCESS(cb,
> +			NETWORK_REGISTRATION_STATUS_NOT_REGISTERED, user_data);
> +	}
> +

This seems questionable to me.  Why not query the attached status like 
the core wants you to?

> +	cbd->user = gprs;
>  	if (qmi_service_send(data->nas, QMI_NAS_GET_SS_INFO, NULL,
>  					get_ss_info_cb, cbd, g_free) > 0)
>  		return;
>

Regards,
-Denis

Re: [PATCH qmi lte v4 5/6] qmi: add LTE atom driver

[Jonas Bonn]

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

Hi Denis,

On 04/20/2017 07:03 PM, Denis Kenzior wrote:
> Hi Jonas,
>
> On 04/18/2017 02:50 AM, Jonas Bonn wrote:
>> +    if (strlen(info->apn) > 0)
>> +        ldd->apn = g_strdup(info->apn);
>> +    else
>> +        ldd->apn = NULL;
>> +
>
> Why do you want to store the APN inside the driver data?

Just to do _something_ with the APN until the FIXME below gets fixed.  
There's no reason to store it in the long run.

>
>> +    /*
>> +     * FIXME: The above is not sufficient.  The APN needs to be set
>> +     * to the 'default' profile so that the modem uses it when
>> +     * connecting the default bearer.
>> +     */
>> +    ofono_warn("LTE APN needs to be written to default profile");


>> +
>> +    qmi_service_create(device, QMI_SERVICE_WDS,
>> +                    create_wds_cb, lte, NULL);
>> +
>
> So I talked to Marcel about this and it seems a QMI service cannot be 
> created multiple times for the same device.  So I think this should 
> fail and you should be using qmi_service_create_shared instead.

OK, thanks for noticing this.  I was, in fact, wondering about this, too.

The QMI changes that I have made in the last few weeks have created the 
same service in multiple atoms (gprs, network-registration, 
gprs-context).  That _does_ actually work on my EC21 which is a pretty 
new Qualcomm chip.  But if older chips have problems with this then I 
suppose this will need to be fixed in the other atoms, too.  I will do so.

/Jonas