All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH lte 0/8] QMI LTE series
@ 2017-04-11  8:18 Jonas Bonn
  2017-04-11  8:18 ` [PATCH 1/8] qmi: NAS definitions adjustment Jonas Bonn
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

LTE is different from other 'GPRS' technologies in that a default bearer
always gets set up when the modem registers to the network.  Ofono has
some support for this and this series tries to set this up for QMI
modems.

This seems to work, but I think that this whole process needs to be
re-thought a bit.  It's far from obvious how this works.  The GPRS
atom is still responsible for maintaining some form of "online" status,
but it does so in a round-about way where set_attached never gets called
because LTE doesn't have an "attached" state like GPRS does.

Other open questions:

What happens when the modem moves from LTE to UMTS network, or from
UMTS network to LTE when a context already has been set up?  Should the
default bearer still be set up on LTE?  Or should the default bearer be
taken down when moving away from LTE?

Jonas Bonn (8):
  qmi: NAS definitions adjustment
  qmi: add WDS parameter definition
  qmi: retrieve GPRS context parameters
  qmi: implement read_settings for automatic contexts
  lte: activate default bearer on network registration
  qmi: add LTE atom driver
  qmi: watch for automatic context activation
  gobi: add LTE atom

 Makefile.am                     |   1 +
 drivers/qmimodem/gprs-context.c |  78 ++++++++++++++
 drivers/qmimodem/gprs.c         |  90 ++++++++++++++++-
 drivers/qmimodem/lte.c          | 219 ++++++++++++++++++++++++++++++++++++++++
 drivers/qmimodem/nas.h          |  14 ++-
 drivers/qmimodem/qmimodem.c     |   2 +
 drivers/qmimodem/qmimodem.h     |   3 +
 drivers/qmimodem/wds.h          |   2 +
 include/lte.h                   |   2 +
 plugins/gobi.c                  |   3 +
 src/lte.c                       | 115 +++++++++++++++++++++
 11 files changed, 525 insertions(+), 4 deletions(-)
 create mode 100644 drivers/qmimodem/lte.c

-- 
2.9.3


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

* [PATCH 1/8] qmi: NAS definitions adjustment
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  2017-04-11 15:23   ` Denis Kenzior
  2017-04-11  8:18 ` [PATCH 2/8] qmi: add WDS parameter definition Jonas Bonn
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

Calling the ps_state/cs_state alternatives *ATTACH_STATUS* was confusing
because there is also a status field in the *serving_system structure.
This patch does a minor rename and adds the appropriate definitions for
the status field.
---
 drivers/qmimodem/gprs.c |  2 +-
 drivers/qmimodem/nas.h  | 14 +++++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
index 983223e..05ad4bd 100644
--- a/drivers/qmimodem/gprs.c
+++ b/drivers/qmimodem/gprs.c
@@ -47,7 +47,7 @@ static bool extract_ss_info(struct qmi_result *result, int *status)
 	if (!ss)
 		return false;
 
-	if (ss->ps_state == QMI_NAS_ATTACH_STATUS_ATTACHED)
+	if (ss->ps_state == QMI_NAS_ATTACH_STATE_ATTACHED)
 		*status = 0x01;
 	else
 		*status = 0x00;
diff --git a/drivers/qmimodem/nas.h b/drivers/qmimodem/nas.h
index dee9d70..9bb9a9a 100644
--- a/drivers/qmimodem/nas.h
+++ b/drivers/qmimodem/nas.h
@@ -140,9 +140,17 @@ struct qmi_nas_current_plmn {
 #define QMI_NAS_RESULT_LOCATION_AREA_CODE	0x1d	/* uint16 */
 #define QMI_NAS_RESULT_CELL_ID			0x1e	/* uint32 */
 
-#define QMI_NAS_ATTACH_STATUS_INVALID		0x00
-#define QMI_NAS_ATTACH_STATUS_ATTACHED		0x01
-#define QMI_NAS_ATTACH_STATUS_DETACHED		0x02
+/* qmi_nas_serving_system.status */
+#define QMI_NAS_REGISTRATION_STATE_NOT_REGISTERED	0x00
+#define QMI_NAS_REGISTRATION_STATE_REGISTEREID		0x01
+#define QMI_NAS_REGISTRATION_STATE_SEARCHING		0x02
+#define QMI_NAS_REGISTRATION_STATE_DENIED		0x03
+#define QMI_NAS_REGISTRATION_STATE_UNKNOWN		0x04
+
+/* cs_state/ps_state */
+#define QMI_NAS_ATTACH_STATE_INVALID		0x00
+#define QMI_NAS_ATTACH_STATE_ATTACHED		0x01
+#define QMI_NAS_ATTACH_STATE_DETACHED		0x02
 
 /* Get info about home network */
 #define QMI_NAS_RESULT_HOME_NETWORK		0x01
-- 
2.9.3


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

* [PATCH 2/8] qmi: add WDS parameter definition
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
  2017-04-11  8:18 ` [PATCH 1/8] qmi: NAS definitions adjustment Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  2017-04-11 16:23   ` Denis Kenzior
  2017-04-11  8:18 ` [PATCH 3/8] qmi: retrieve GPRS context parameters Jonas Bonn
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

---
 drivers/qmimodem/wds.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/qmimodem/wds.h b/drivers/qmimodem/wds.h
index 4843f92..e03d177 100644
--- a/drivers/qmimodem/wds.h
+++ b/drivers/qmimodem/wds.h
@@ -62,6 +62,7 @@ struct qmi_wds_notify_conn_status {
 #define QMI_WDS_RESULT_SECONDARY_DNS		0x16	/* uint32 IPv4 */
 #define QMI_WDS_RESULT_IP_ADDRESS		0x1e	/* uint32 IPv4 */
 #define QMI_WDS_RESULT_GATEWAY			0x20	/* uint32 IPv4 */
+#define QMI_WDS_RESULT_GATEWAY_NETMASK		0x21	/* uint32 IPv4 */
 #define QMI_WDS_RESULT_IP_FAMILY		0x2b	/* uint8 */
 
 #define QMI_WDS_PDP_TYPE_IPV4			0x00
-- 
2.9.3


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

* [PATCH 3/8] qmi: retrieve GPRS context parameters
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
  2017-04-11  8:18 ` [PATCH 1/8] qmi: NAS definitions adjustment Jonas Bonn
  2017-04-11  8:18 ` [PATCH 2/8] qmi: add WDS parameter definition Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  2017-04-11 16:44   ` Denis Kenzior
  2017-04-11  8:18 ` [PATCH 4/8] qmi: implement read_settings for automatic contexts Jonas Bonn
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

The GPRS context needs to be configured with connection parameters when
the bearer has been established.  This was only partially implemented, so
this patch adds additional parameters to those passed to the context.
---
 drivers/qmimodem/gprs-context.c | 52 +++++++++++++++++++++++++++++++++++++++++
 drivers/qmimodem/wds.h          |  1 +
 2 files changed, 53 insertions(+)

diff --git a/drivers/qmimodem/gprs-context.c b/drivers/qmimodem/gprs-context.c
index da2be24..1ae2a5a 100644
--- a/drivers/qmimodem/gprs-context.c
+++ b/drivers/qmimodem/gprs-context.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <string.h>
+#include <arpa/inet.h>
 
 #include <ofono/log.h>
 #include <ofono/modem.h>
@@ -78,18 +79,67 @@ static void get_settings_cb(struct qmi_result *result, void *user_data)
 	struct ofono_modem *modem;
 	const char *interface;
 	uint8_t pdp_type, ip_family;
+	uint32_t ip_addr;
+	struct in_addr addr;
+	char* straddr;
+	char* apn = NULL;
+	const char *dns[3] = { NULL, NULL, NULL };
 
 	DBG("");
 
 	if (qmi_result_set_error(result, NULL))
 		goto done;
 
+	apn = qmi_result_get_string(result, QMI_WDS_RESULT_APN);
+	if (apn) {
+		DBG("APN: %s", apn);
+	}
 	if (qmi_result_get_uint8(result, QMI_WDS_RESULT_PDP_TYPE, &pdp_type))
 		DBG("PDP type %d", pdp_type);
 
 	if (qmi_result_get_uint8(result, QMI_WDS_RESULT_IP_FAMILY, &ip_family))
 		DBG("IP family %d", ip_family);
 
+	if (qmi_result_get_uint32(result,QMI_WDS_RESULT_IP_ADDRESS, &ip_addr)) {
+
+		addr.s_addr = htonl(ip_addr);
+		straddr = inet_ntoa(addr);
+		DBG("IP addr: %s", straddr);
+		ofono_gprs_context_set_ipv4_address(gc, straddr, 1);
+	}
+	if (qmi_result_get_uint32(result,QMI_WDS_RESULT_GATEWAY, &ip_addr)) {
+
+		addr.s_addr = htonl(ip_addr);
+		straddr = inet_ntoa(addr);
+		DBG("Gateway: %s", straddr);
+		ofono_gprs_context_set_ipv4_gateway(gc, straddr);
+	}
+	if (qmi_result_get_uint32(result,
+				QMI_WDS_RESULT_GATEWAY_NETMASK, &ip_addr)) {
+
+		addr.s_addr = htonl(ip_addr);
+		straddr = inet_ntoa(addr);
+		DBG("Gateway netmask: %s", straddr);
+		ofono_gprs_context_set_ipv4_netmask(gc, straddr);
+	}
+	if (qmi_result_get_uint32(result,
+				QMI_WDS_RESULT_PRIMARY_DNS, &ip_addr)) {
+
+		addr.s_addr = htonl(ip_addr);
+		dns[0] = inet_ntoa(addr);
+		DBG("Primary DNS: %s", dns[0]);
+	}
+	if (qmi_result_get_uint32(result,
+				QMI_WDS_RESULT_SECONDARY_DNS, &ip_addr)) {
+
+		addr.s_addr = htonl(ip_addr);
+		dns[1] = inet_ntoa(addr);
+		DBG("Secondary DNS: %s", dns[1]);
+	}
+
+	if (dns[0])
+		ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
+
 done:
 	modem = ofono_gprs_context_get_modem(gc);
 	interface = ofono_modem_get_string(modem, "NetworkInterface");
@@ -98,6 +148,8 @@ done:
 
 	CALLBACK_WITH_SUCCESS(cb, cbd->data);
 
+	g_free(apn);
+
 	g_free(cbd);
 }
 
diff --git a/drivers/qmimodem/wds.h b/drivers/qmimodem/wds.h
index e03d177..8aae996 100644
--- a/drivers/qmimodem/wds.h
+++ b/drivers/qmimodem/wds.h
@@ -58,6 +58,7 @@ struct qmi_wds_notify_conn_status {
 
 /* Get the runtime data session settings */
 #define QMI_WDS_RESULT_PDP_TYPE			0x11	/* uint8 */
+#define QMI_WDS_RESULT_APN			0x14	/* string */
 #define QMI_WDS_RESULT_PRIMARY_DNS		0x15	/* uint32 IPv4 */
 #define QMI_WDS_RESULT_SECONDARY_DNS		0x16	/* uint32 IPv4 */
 #define QMI_WDS_RESULT_IP_ADDRESS		0x1e	/* uint32 IPv4 */
-- 
2.9.3


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

* [PATCH 4/8] qmi: implement read_settings for automatic contexts
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
                   ` (2 preceding siblings ...)
  2017-04-11  8:18 ` [PATCH 3/8] qmi: retrieve GPRS context parameters Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  2017-04-11 17:02   ` Denis Kenzior
  2017-04-11  8:18 ` [PATCH 5/8] lte: activate default bearer on network registration Jonas Bonn
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

For LTE, a context is created automatically when the modem registers
to the network.  The read_settings function is called for these
automatic contexts to get their configuration.
---
 drivers/qmimodem/gprs-context.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/qmimodem/gprs-context.c b/drivers/qmimodem/gprs-context.c
index 1ae2a5a..3841c7e 100644
--- a/drivers/qmimodem/gprs-context.c
+++ b/drivers/qmimodem/gprs-context.c
@@ -153,6 +153,31 @@ done:
 	g_free(cbd);
 }
 
+static void qmi_gprs_read_settings(struct ofono_gprs_context* gc,
+					unsigned int cid,
+					ofono_gprs_context_cb_t cb,
+					void *user_data)
+{
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+	struct gprs_context_data *data = ofono_gprs_context_get_data(gc);
+
+	DBG("cid %u", cid);
+
+	data->active_context = cid;
+
+	cbd->user = gc;
+
+	if (qmi_service_send(data->wds, QMI_WDS_GET_SETTINGS, NULL,
+					get_settings_cb, cbd, NULL) > 0)
+		return;
+
+	data->active_context = 0;
+
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+	g_free(cbd);
+}
+
 static void start_net_cb(struct qmi_result *result, void *user_data)
 {
 	struct cb_data *cbd = user_data;
@@ -450,6 +475,7 @@ static struct ofono_gprs_context_driver driver = {
 	.remove			= qmi_gprs_context_remove,
 	.activate_primary	= qmi_activate_primary,
 	.deactivate_primary	= qmi_deactivate_primary,
+	.read_settings		= qmi_gprs_read_settings,
 };
 
 void qmi_gprs_context_init(void)
-- 
2.9.3


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

* [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
                   ` (3 preceding siblings ...)
  2017-04-11  8:18 ` [PATCH 4/8] qmi: implement read_settings for automatic contexts Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  2017-04-11 17:07   ` Denis Kenzior
  2017-04-11  8:18 ` [PATCH 6/8] qmi: add LTE atom driver Jonas Bonn
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

The LTE atom should watch the network status and automatically activate
the default bearer when the modem becomes registered to the LTE network.

This patch adds a device callback, activate_default_bearer(), to allow
the device to start networking.
---
 include/lte.h |   2 +
 src/lte.c     | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/include/lte.h b/include/lte.h
index f3ff405..9dc0109 100644
--- a/include/lte.h
+++ b/include/lte.h
@@ -43,6 +43,8 @@ struct ofono_lte_driver {
 	void (*set_default_attach_info)(const struct ofono_lte *lte,
 			const struct ofono_lte_default_attach_info *info,
 			ofono_lte_cb_t cb, void *data);
+	void (*activate_default_bearer)(struct ofono_lte *lte,
+			ofono_lte_cb_t cb, void *data);
 };
 
 int ofono_lte_driver_register(const struct ofono_lte_driver *d);
diff --git a/src/lte.c b/src/lte.c
index 70e0c18..3c6a0f8 100644
--- a/src/lte.c
+++ b/src/lte.c
@@ -37,11 +37,15 @@
 #include "common.h"
 #include "storage.h"
 
+#define LTE_FLAG_ACTIVATING 0x1
+#define LTE_FLAG_RECHECK 0x2
+
 #define SETTINGS_STORE "lte"
 #define SETTINGS_GROUP "Settings"
 #define DEFAULT_APN_KEY "DefaultAccessPointName"
 
 struct ofono_lte {
+	int flags;
 	const struct ofono_lte_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
@@ -50,10 +54,117 @@ struct ofono_lte {
 	DBusMessage *pending;
 	struct ofono_lte_default_attach_info pending_info;
 	struct ofono_lte_default_attach_info info;
+	int netreg_status;
+	struct ofono_netreg *netreg;
+	unsigned int netreg_watch;
+	unsigned int status_watch;
+	ofono_bool_t online;
+	ofono_bool_t roaming_allowed;
 };
 
 static GSList *g_drivers = NULL;
 
+static void lte_netreg_removed(struct ofono_lte *lte)
+{
+	lte->netreg = NULL;
+
+	lte->status_watch = 0;
+	lte->netreg_status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
+	lte->online = 0;
+
+//	lte_netreg_update(lte);
+}
+
+static void lte_netreg_update(struct ofono_lte *lte);
+
+static void activate_default_bearer_cb(const struct ofono_error* error,
+					void *data)
+{
+	struct ofono_lte *lte = data;
+
+	DBG("error %d", error->type);
+
+	lte->flags &= ~LTE_FLAG_ACTIVATING;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_lte_remove(lte);
+		return;
+	}
+
+	lte->online = TRUE;
+
+	if (lte->flags & LTE_FLAG_RECHECK) {
+		lte->flags &= ~LTE_FLAG_RECHECK;
+		lte_netreg_update(lte);
+	}
+}
+
+static void lte_netreg_update(struct ofono_lte *lte)
+{
+	ofono_bool_t attach;
+
+	DBG("");
+
+	attach = lte->netreg_status == NETWORK_REGISTRATION_STATUS_REGISTERED;
+
+	attach = attach || (lte->roaming_allowed &&
+		lte->netreg_status == NETWORK_REGISTRATION_STATUS_ROAMING);
+
+	if (!attach)
+		return;
+
+	if (lte->online)
+		return;
+
+	if (ofono_netreg_get_technology(lte->netreg) !=
+			ACCESS_TECHNOLOGY_EUTRAN)
+		return;
+
+	if (lte->flags & LTE_FLAG_ACTIVATING) {
+		lte->flags |= LTE_FLAG_RECHECK;
+		return;
+	}
+
+	if (lte->driver->activate_default_bearer) {
+		lte->flags |= LTE_FLAG_ACTIVATING;
+		lte->driver->activate_default_bearer(lte,
+				activate_default_bearer_cb, lte);
+	}
+}
+
+static void netreg_status_changed(int status, int lac, int ci, int tech,
+					const char *mcc, const char *mnc,
+					void *data)
+{
+	struct ofono_lte *lte = data;
+
+	DBG("%d", status);
+
+	lte->netreg_status = status;
+
+	lte_netreg_update(lte);
+}
+
+
+static void netreg_watch(struct ofono_atom *atom,
+				enum ofono_atom_watch_condition cond,
+				void *data)
+{
+	struct ofono_lte *lte = data;
+
+	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
+		lte_netreg_removed(lte);
+		return;
+	}
+
+	lte->netreg = __ofono_atom_get_data(atom);
+	lte->netreg_status = ofono_netreg_get_status(lte->netreg);
+	lte->status_watch = __ofono_netreg_add_status_watch(lte->netreg,
+					netreg_status_changed, lte, NULL);
+
+	lte_netreg_update(lte);
+}
+
 static void lte_load_settings(struct ofono_lte *lte)
 {
 	char *apn;
@@ -327,6 +438,10 @@ static void ofono_lte_finish_register(struct ofono_lte *lte)
 
 	ofono_modem_add_interface(modem, OFONO_LTE_INTERFACE);
 
+	lte->netreg_watch = __ofono_modem_add_atom_watch(modem,
+					OFONO_ATOM_TYPE_NETREG,
+					netreg_watch, lte, NULL);
+
 	__ofono_atom_register(lte->atom, lte_atom_unregister);
 }
 
-- 
2.9.3


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

* [PATCH 6/8] qmi: add LTE atom driver
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
                   ` (4 preceding siblings ...)
  2017-04-11  8:18 ` [PATCH 5/8] lte: activate default bearer on network registration Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  2017-04-11 17:09   ` Denis Kenzior
  2017-04-11  8:18 ` [PATCH 7/8] qmi: watch for automatic context activation Jonas Bonn
  2017-04-11  8:18 ` [PATCH 8/8] gobi: add LTE atom Jonas Bonn
  7 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

When the LTE atom detects that the modem is registered to an LTE network,
the driver will be asked to activate the default bearer.  For QMI modems,
this means that the "Start Network" method needs to be called with the
default APN.
---
 Makefile.am                 |   1 +
 drivers/qmimodem/lte.c      | 219 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/qmimodem/qmimodem.c |   2 +
 drivers/qmimodem/qmimodem.h |   3 +
 4 files changed, 225 insertions(+)
 create mode 100644 drivers/qmimodem/lte.c

diff --git a/Makefile.am b/Makefile.am
index b232f97..a7767f9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -231,6 +231,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..71a323f
--- /dev/null
+++ b/drivers/qmimodem/lte.c
@@ -0,0 +1,219 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2016  Endocode AG. 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;
+	uint32_t default_bearer_handle;
+};
+
+static void start_net_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_lte_cb_t cb = cbd->cb;
+	const struct ofono_lte *lte = cbd->user;
+	struct lte_data *ldd = ofono_lte_get_data(lte);
+
+	uint32_t handle;
+
+	DBG("");
+
+	if (qmi_result_set_error(result, NULL)) {
+		goto error;
+	}
+
+	if (!qmi_result_get_uint32(result,
+				QMI_WDS_RESULT_PKT_HANDLE, &handle)) {
+		goto error;
+	}
+
+	DBG("packet handle 0x%08x", handle);
+
+	ldd->default_bearer_handle = handle;
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+
+	g_free(cbd);
+
+	return;
+
+error:
+	ldd->default_bearer_handle = 0;
+
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+	g_free(cbd);
+}
+
+static void qmi_lte_activate_default_bearer(struct ofono_lte *lte,
+			ofono_lte_cb_t cb, void * data)
+{
+	struct lte_data *ldd = ofono_lte_get_data(lte);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	struct qmi_param *param = NULL;
+
+	DBG("");
+
+	cbd->user = lte;
+
+	if (ldd->apn) {
+		param = qmi_param_new();
+		qmi_param_append(param, QMI_WDS_PARAM_APN,
+					strlen(ldd->apn), ldd->apn);
+		DBG("Default APN: %s", ldd->apn);
+	}
+
+	if (qmi_service_send(ldd->wds, QMI_WDS_START_NET, param,
+					start_net_cb, cbd, NULL) > 0)
+		return;
+
+	qmi_param_free(param);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+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);
+
+	if (ldd->apn)
+		g_free(ldd->apn);
+
+	if (strlen(info->apn) > 0)
+		ldd->apn = g_strdup(info->apn);
+	else
+		ldd->apn = NULL;
+
+	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);
+//	struct qmi_param *param;
+
+	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);
+}
+
+#if 0
+static gboolean lte_delayed_register(gpointer user_data)
+{
+	struct ofono_lte *lte = user_data;
+
+	qmi_service_create(device, QMI_SERVICE_WDS,
+					create_wds_cb, lte, NULL);
+
+	return FALSE;
+}
+#endif
+
+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);
+
+//	g_idle_add(lte_delayed_register, lte);
+
+	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);
+
+	if (ldd->apn)
+		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,
+	.activate_default_bearer	= qmi_lte_activate_default_bearer,
+};
+
+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


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

* [PATCH 7/8] qmi: watch for automatic context activation
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
                   ` (5 preceding siblings ...)
  2017-04-11  8:18 ` [PATCH 6/8] qmi: add LTE atom driver Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  2017-04-11  8:18 ` [PATCH 8/8] gobi: add LTE atom Jonas Bonn
  7 siblings, 0 replies; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

For LTE, a default bearer gets set up as soon as the modem registers to
the network.  The GPRS atom needs to detect the presence of this bearer
in order to set up the appropriate context atom.

This is all a bit confusing but it's a result of the way ofono is
currently architected.  The GPRS atom serves two purposes:  to "attach"
to the network, and to hold "online" (attached) state for the purpose
of communication with the connection manager (via DBus).  For LTE, there
is no concept of being "attached"; when the modem is online, a default
bearer is automatically set so "attached" becomes synonymous with
attached-and-context-activated.  Context activation thus becomes "attached"
in LTE, but this information still needs to be held in the GPRS atom
which otherwise has no purpose.
---
 drivers/qmimodem/gprs.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
index 05ad4bd..d4ced1c 100644
--- a/drivers/qmimodem/gprs.c
+++ b/drivers/qmimodem/gprs.c
@@ -29,13 +29,76 @@
 
 #include "qmi.h"
 #include "nas.h"
+#include "wds.h"
 
 #include "qmimodem.h"
 
 struct gprs_data {
 	struct qmi_service *nas;
+	struct qmi_service *wds;
+	int default_bearer_cid;
 };
 
+static void get_settings_cb(struct qmi_result *result, void *user_data)
+{
+	struct ofono_gprs *gprs = user_data;
+	struct gprs_data *data = ofono_gprs_get_data(gprs);
+	char* apn = NULL;
+
+	DBG("");
+
+	if (qmi_result_set_error(result, NULL))
+		goto done;
+
+	apn = qmi_result_get_string(result, QMI_WDS_RESULT_APN);
+	if (apn) {
+		DBG("APN: %s", apn);
+	}
+
+done:
+	if (!apn)
+		apn = g_strdup("");
+
+	ofono_gprs_cid_activated(gprs, data->default_bearer_cid, apn);
+
+	g_free(apn);
+}
+
+static void pkt_status_notify(struct qmi_result *result, void *user_data)
+{
+	struct ofono_gprs *gprs = user_data;
+	struct gprs_data *data = ofono_gprs_get_data(gprs);
+	const struct qmi_wds_notify_conn_status *status;
+	uint16_t len;
+
+	DBG("");
+
+	status = qmi_result_get(result, QMI_WDS_NOTIFY_CONN_STATUS, &len);
+	if (!status)
+		return;
+
+	DBG("conn status %d", status->status);
+
+	switch (status->status) {
+	case QMI_WDS_CONN_STATUS_CONNECTED:
+		DBG("connection status connected");
+		if (!data->default_bearer_cid)
+			data->default_bearer_cid = 1;
+
+		if (qmi_service_send(data->wds, QMI_WDS_GET_SETTINGS, NULL,
+					get_settings_cb, gprs, NULL) > 0)
+			return;
+
+		ofono_gprs_cid_activated(gprs, data->default_bearer_cid, "");
+		break;
+	case QMI_WDS_CONN_STATUS_DISCONNECTED:
+		data->default_bearer_cid = 0;
+		DBG("connection status disconnected");
+		break;
+	}
+}
+
+
 static bool extract_ss_info(struct qmi_result *result, int *status)
 {
 	const struct qmi_nas_serving_system *ss;
@@ -159,6 +222,28 @@ static void qmi_attached_status(struct ofono_gprs *gprs,
 	g_free(cbd);
 }
 
+static void create_wds_cb(struct qmi_service *service, void *user_data)
+{
+	struct ofono_gprs *gprs = user_data;
+	struct gprs_data *data = ofono_gprs_get_data(gprs);
+
+	DBG("");
+
+	if (!service) {
+		ofono_error("Failed to request WDS service");
+		ofono_gprs_remove(gprs);
+		return;
+	}
+
+	data->wds = qmi_service_ref(service);
+
+	qmi_service_send(data->wds, QMI_WDS_GET_PKT_STATUS, NULL,
+					pkt_status_notify, gprs, NULL);
+
+	qmi_service_register(data->wds, QMI_WDS_PKT_STATUS_IND,
+					pkt_status_notify, gprs, NULL);
+}
+
 static void create_nas_cb(struct qmi_service *service, void *user_data)
 {
 	struct ofono_gprs *gprs = user_data;
@@ -202,6 +287,7 @@ static int qmi_gprs_probe(struct ofono_gprs *gprs,
 	ofono_gprs_set_data(gprs, data);
 
 	qmi_service_create(device, QMI_SERVICE_NAS, create_nas_cb, gprs, NULL);
+	qmi_service_create(device, QMI_SERVICE_WDS, create_wds_cb, gprs, NULL);
 
 	return 0;
 }
@@ -215,8 +301,10 @@ static void qmi_gprs_remove(struct ofono_gprs *gprs)
 	ofono_gprs_set_data(gprs, NULL);
 
 	qmi_service_unregister_all(data->nas);
+	qmi_service_unregister_all(data->wds);
 
 	qmi_service_unref(data->nas);
+	qmi_service_unref(data->wds);
 
 	g_free(data);
 }
-- 
2.9.3


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

* [PATCH 8/8] gobi: add LTE atom
  2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
                   ` (6 preceding siblings ...)
  2017-04-11  8:18 ` [PATCH 7/8] qmi: watch for automatic context activation Jonas Bonn
@ 2017-04-11  8:18 ` Jonas Bonn
  7 siblings, 0 replies; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11  8:18 UTC (permalink / raw)
  To: ofono

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

---
 plugins/gobi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index df35f94..58df2ce 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>
@@ -506,6 +507,8 @@ static void gobi_post_online(struct ofono_modem *modem)
 		if (gprs && gc)
 			ofono_gprs_add_context(gprs, gc);
 	}
+
+	ofono_lte_create(modem, "qmimodem", data->device);
 }
 
 static struct ofono_modem_driver gobi_driver = {
-- 
2.9.3


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

* Re: [PATCH 1/8] qmi: NAS definitions adjustment
  2017-04-11  8:18 ` [PATCH 1/8] qmi: NAS definitions adjustment Jonas Bonn
@ 2017-04-11 15:23   ` Denis Kenzior
  0 siblings, 0 replies; 22+ messages in thread
From: Denis Kenzior @ 2017-04-11 15:23 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

On 04/11/2017 03:18 AM, Jonas Bonn wrote:
> Calling the ps_state/cs_state alternatives *ATTACH_STATUS* was confusing
> because there is also a status field in the *serving_system structure.
> This patch does a minor rename and adds the appropriate definitions for
> the status field.
> ---
>  drivers/qmimodem/gprs.c |  2 +-
>  drivers/qmimodem/nas.h  | 14 +++++++++++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
>

Applied, thanks.

Regards,
-Denis


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

* Re: [PATCH 2/8] qmi: add WDS parameter definition
  2017-04-11  8:18 ` [PATCH 2/8] qmi: add WDS parameter definition Jonas Bonn
@ 2017-04-11 16:23   ` Denis Kenzior
  0 siblings, 0 replies; 22+ messages in thread
From: Denis Kenzior @ 2017-04-11 16:23 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

On 04/11/2017 03:18 AM, Jonas Bonn wrote:
> ---
>  drivers/qmimodem/wds.h | 1 +
>  1 file changed, 1 insertion(+)
>

Applied, thanks.

Regards,
-Denis


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

* Re: [PATCH 3/8] qmi: retrieve GPRS context parameters
  2017-04-11  8:18 ` [PATCH 3/8] qmi: retrieve GPRS context parameters Jonas Bonn
@ 2017-04-11 16:44   ` Denis Kenzior
  0 siblings, 0 replies; 22+ messages in thread
From: Denis Kenzior @ 2017-04-11 16:44 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

On 04/11/2017 03:18 AM, Jonas Bonn wrote:
> The GPRS context needs to be configured with connection parameters when
> the bearer has been established.  This was only partially implemented, so
> this patch adds additional parameters to those passed to the context.
> ---
>  drivers/qmimodem/gprs-context.c | 52 +++++++++++++++++++++++++++++++++++++++++
>  drivers/qmimodem/wds.h          |  1 +
>  2 files changed, 53 insertions(+)
>

I went ahead and applied a slightly modified version of this patch that 
fixed various style issues.

Regards,
-Denis

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

* Re: [PATCH 4/8] qmi: implement read_settings for automatic contexts
  2017-04-11  8:18 ` [PATCH 4/8] qmi: implement read_settings for automatic contexts Jonas Bonn
@ 2017-04-11 17:02   ` Denis Kenzior
  0 siblings, 0 replies; 22+ messages in thread
From: Denis Kenzior @ 2017-04-11 17:02 UTC (permalink / raw)
  To: ofono

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

On 04/11/2017 03:18 AM, Jonas Bonn wrote:
> For LTE, a context is created automatically when the modem registers
> to the network.  The read_settings function is called for these
> automatic contexts to get their configuration.
> ---
>  drivers/qmimodem/gprs-context.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>

I went ahead and applied this patch, but see below:

> diff --git a/drivers/qmimodem/gprs-context.c b/drivers/qmimodem/gprs-context.c
> index 1ae2a5a..3841c7e 100644
> --- a/drivers/qmimodem/gprs-context.c
> +++ b/drivers/qmimodem/gprs-context.c
> @@ -153,6 +153,31 @@ done:
>  	g_free(cbd);
>  }
>
> +static void qmi_gprs_read_settings(struct ofono_gprs_context* gc,
> +					unsigned int cid,
> +					ofono_gprs_context_cb_t cb,
> +					void *user_data)
> +{
> +	struct cb_data *cbd = cb_data_new(cb, user_data);
> +	struct gprs_context_data *data = ofono_gprs_context_get_data(gc);
> +
> +	DBG("cid %u", cid);
> +
> +	data->active_context = cid;
> +
> +	cbd->user = gc;
> +
> +	if (qmi_service_send(data->wds, QMI_WDS_GET_SETTINGS, NULL,
> +					get_settings_cb, cbd, NULL) > 0)

You really should be setting the qmi_destroy_func_t for 
qmi_service_send.  This saves you a g_free call inside get_settings_cb, 
but more importantly it avoids a memory leak in case the 
qmi_service/qmi_device is destroyed before the callback has been called.

I went ahead and fixed this in the upstream code in commits:
f29a316c918bfd67072bdb4f46ca90a3b8954108
7a2e198fd7430c468f2a15ff82467c18c649ddf3

But there may be other instances of this bug lurking around.

> +		return;
> +
> +	data->active_context = 0;
> +
> +	CALLBACK_WITH_FAILURE(cb, cbd->data);
> +
> +	g_free(cbd);
> +}
> +
>  static void start_net_cb(struct qmi_result *result, void *user_data)
>  {
>  	struct cb_data *cbd = user_data;
> @@ -450,6 +475,7 @@ static struct ofono_gprs_context_driver driver = {
>  	.remove			= qmi_gprs_context_remove,
>  	.activate_primary	= qmi_activate_primary,
>  	.deactivate_primary	= qmi_deactivate_primary,
> +	.read_settings		= qmi_gprs_read_settings,
>  };
>
>  void qmi_gprs_context_init(void)
>

Regards,
-Denis

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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-11  8:18 ` [PATCH 5/8] lte: activate default bearer on network registration Jonas Bonn
@ 2017-04-11 17:07   ` Denis Kenzior
  2017-04-11 20:48     ` Jonas Bonn
  0 siblings, 1 reply; 22+ messages in thread
From: Denis Kenzior @ 2017-04-11 17:07 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

On 04/11/2017 03:18 AM, Jonas Bonn wrote:
> The LTE atom should watch the network status and automatically activate
> the default bearer when the modem becomes registered to the LTE network.
>
> This patch adds a device callback, activate_default_bearer(), to allow
> the device to start networking.

Nope, we're not doing it this way.  Once we're attached to LTE there's 
no need to activate a default bearer.  It is already active.  The modem 
should tell us that this is the case.

Another issue is that the network is free to ignore our default bearer 
setting, so you might be activating (or trying to) yet another context.

Doesn't QMI provide an equivalent of +CGEV?

Regards,
-Denis

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

* Re: [PATCH 6/8] qmi: add LTE atom driver
  2017-04-11  8:18 ` [PATCH 6/8] qmi: add LTE atom driver Jonas Bonn
@ 2017-04-11 17:09   ` Denis Kenzior
  0 siblings, 0 replies; 22+ messages in thread
From: Denis Kenzior @ 2017-04-11 17:09 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

> +++ b/drivers/qmimodem/lte.c
> @@ -0,0 +1,219 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2016  Endocode AG. All rights reserved.
> + *

You might want to update the Copyright when copy-pasting stuff.

Regards,
-Denis

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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-11 17:07   ` Denis Kenzior
@ 2017-04-11 20:48     ` Jonas Bonn
  2017-04-11 21:21       ` Denis Kenzior
  0 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-11 20:48 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

On 04/11/2017 07:07 PM, Denis Kenzior wrote:
> On 04/11/2017 03:18 AM, Jonas Bonn wrote:
>> The LTE atom should watch the network status and automatically activate
>> the default bearer when the modem becomes registered to the LTE network.
>>
>> This patch adds a device callback, activate_default_bearer(), to allow
>> the device to start networking.
>
> Nope, we're not doing it this way.  Once we're attached to LTE there's 
> no need to activate a default bearer.  It is already active.  The 
> modem should tell us that this is the case.
>
> Another issue is that the network is free to ignore our default bearer 
> setting, so you might be activating (or trying to) yet another context.
>
> Doesn't QMI provide an equivalent of +CGEV?

I haven't found an equivalent.  Right now it looks to me as though the 
only way to set up a PDP context is to call the "Start Networking" 
method.  If I don't start networking, the packet status callback never 
gives me a "connected" status.  It's not enough to just power on the 
modem... although that does get me registered to the network.

Furthermore, it doesn't matter what APN I use, I always get a usable 
data-connection.  "Get Settings" never returns an APN string, either, 
which makes me that the network is ignoring my setting altogether and 
just giving me a default bearer...???

If anyone knows how this is supposed to work, I'd appreciate some guidance!

/Jonas

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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-11 20:48     ` Jonas Bonn
@ 2017-04-11 21:21       ` Denis Kenzior
  2017-04-12 14:28         ` Jonas Bonn
  0 siblings, 1 reply; 22+ messages in thread
From: Denis Kenzior @ 2017-04-11 21:21 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

On 04/11/2017 03:48 PM, Jonas Bonn wrote:
> Hi Denis,
>
> On 04/11/2017 07:07 PM, Denis Kenzior wrote:
>> On 04/11/2017 03:18 AM, Jonas Bonn wrote:
>>> The LTE atom should watch the network status and automatically activate
>>> the default bearer when the modem becomes registered to the LTE network.
>>>
>>> This patch adds a device callback, activate_default_bearer(), to allow
>>> the device to start networking.
>>
>> Nope, we're not doing it this way.  Once we're attached to LTE there's
>> no need to activate a default bearer.  It is already active.  The
>> modem should tell us that this is the case.
>>
>> Another issue is that the network is free to ignore our default bearer
>> setting, so you might be activating (or trying to) yet another context.
>>
>> Doesn't QMI provide an equivalent of +CGEV?
>
> I haven't found an equivalent.  Right now it looks to me as though the
> only way to set up a PDP context is to call the "Start Networking"
> method.  If I don't start networking, the packet status callback never
> gives me a "connected" status.  It's not enough to just power on the
> modem... although that does get me registered to the network.

QMI is an ancient protocol, so maybe it simply didn't get extended for 
LTE.  Or we just don't know how to drive it properly.  Is there an AT 
channel ?  Does that one report anything related to CGEV?

>
> Furthermore, it doesn't matter what APN I use, I always get a usable
> data-connection.  "Get Settings" never returns an APN string, either,
> which makes me that the network is ignoring my setting altogether and
> just giving me a default bearer...???

The network is ultimately responsible for allocating an APN.  It could 
be blank or null or whatever.  Or maybe we don't know the right magic 
incantation.

Regards,
-Denis

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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-11 21:21       ` Denis Kenzior
@ 2017-04-12 14:28         ` Jonas Bonn
  2017-04-12 21:10           ` Jonas Bonn
  0 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-12 14:28 UTC (permalink / raw)
  To: ofono

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

On 04/11/2017 11:21 PM, Denis Kenzior wrote:
> Hi Jonas,
>
> On 04/11/2017 03:48 PM, Jonas Bonn wrote:
>> Hi Denis,
>>
>> On 04/11/2017 07:07 PM, Denis Kenzior wrote:
>>> On 04/11/2017 03:18 AM, Jonas Bonn wrote:
>>>> The LTE atom should watch the network status and automatically 
>>>> activate
>>>> the default bearer when the modem becomes registered to the LTE 
>>>> network.
>>>>
>>>> This patch adds a device callback, activate_default_bearer(), to allow
>>>> the device to start networking.
>>>
>>> Nope, we're not doing it this way.  Once we're attached to LTE there's
>>> no need to activate a default bearer.  It is already active. The
>>> modem should tell us that this is the case.
>>>
>>> Another issue is that the network is free to ignore our default bearer
>>> setting, so you might be activating (or trying to) yet another context.
>>>
>>> Doesn't QMI provide an equivalent of +CGEV?
>>
>> I haven't found an equivalent.  Right now it looks to me as though the
>> only way to set up a PDP context is to call the "Start Networking"
>> method.  If I don't start networking, the packet status callback never
>> gives me a "connected" status.  It's not enough to just power on the
>> modem... although that does get me registered to the network.
>
> QMI is an ancient protocol, so maybe it simply didn't get extended for 
> LTE.  Or we just don't know how to drive it properly.  Is there an AT 
> channel ?  Does that one report anything related to CGEV?
>
>>
>> Furthermore, it doesn't matter what APN I use, I always get a usable
>> data-connection.  "Get Settings" never returns an APN string, either,
>> which makes me that the network is ignoring my setting altogether and
>> just giving me a default bearer...???
>
> The network is ultimately responsible for allocating an APN.  It could 
> be blank or null or whatever.  Or maybe we don't know the right magic 
> incantation.

Alright, I'm going to explain my understanding of how this works:

When the QMI modem registers to an LTE network, it negotiates a default 
bearer using the "default settings".  The default settings are taken 
from the QMI WDS "default profile" which is presumably profile 1 by default.

Even though the network registration has taken place and the bearer is 
in place, the QMI modem doesn't bring up the network interface. Calling 
"get current settings" at this point gives you nothing.

At this point, you need to bring up the network interface by calling 
"Start network".  Start network takes a bunch of parameters but these 
parameters are all ignored since the bearer is already set up.

I haven't tested this, but I suspect that calling "Start network" for 
the same profile again will fail...???  A profile number corresponds to 
a Context ID...???  Maybe...

If you need a specific setting for the default bearer (like APN), you 
need to set it in the default profile before registering to the 
network.  This is what the LTE atom must take care of... and, hence, the 
atom must be created
before setting the modem online.

For non-LTE, the bearer isn't set up until "Start network" is called, so 
the usual context parameters can be passed to it at that time.  Any 
parameters that are omitted are taken from the default profile.

Given all of the above, for QMI this means:

i)  When the netreg atom detects registration to an LTE network then we 
can call "Start network" immediately.
ii)  Start network will give us the packet-handle for the default 
bearer.  This can be held by the netreg atom.
iii)  The packet handle survives until the network becomes unregistered 
(even through a transition to UMTS,GPRS,etc networks).
iv)  There is never any reason to call "Stop network" for the default 
bearer because a network unregistration will achieve the same thing.  If 
the network becomes unregistered we can throw away the packet handle.  
The gprs-context associated with the default bearer won't know the 
packet-handle, so we need to make sure it doesn't try to call "Stop 
network" for it's instance... but that's a detail, I think.

It seems that there exists a method to change the default profile 
number... it's undocumented, though, AFAICT.

What's a bit unclear to me, though, is how this interacts with network 
technology transitions... what happens when I move out of LTE coverage 
and into a UMTS/GPRS network?

Questions?  Feedback appreciated...
/Jonas


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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-12 14:28         ` Jonas Bonn
@ 2017-04-12 21:10           ` Jonas Bonn
  2017-04-13 15:14             ` Denis Kenzior
  0 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-12 21:10 UTC (permalink / raw)
  To: ofono

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

On 04/12/2017 04:28 PM, Jonas Bonn wrote:
> On 04/11/2017 11:21 PM, Denis Kenzior wrote:
>> Hi Jonas,
>>
>> On 04/11/2017 03:48 PM, Jonas Bonn wrote:
>>> Hi Denis,
>>>
>>> On 04/11/2017 07:07 PM, Denis Kenzior wrote:
>>>> On 04/11/2017 03:18 AM, Jonas Bonn wrote:
>>>>> The LTE atom should watch the network status and automatically 
>>>>> activate
>>>>> the default bearer when the modem becomes registered to the LTE 
>>>>> network.
>>>>>
>>>>> This patch adds a device callback, activate_default_bearer(), to 
>>>>> allow
>>>>> the device to start networking.
>>>>
>>>> Nope, we're not doing it this way.  Once we're attached to LTE there's
>>>> no need to activate a default bearer.  It is already active. The
>>>> modem should tell us that this is the case.
>>>>
>>>> Another issue is that the network is free to ignore our default bearer
>>>> setting, so you might be activating (or trying to) yet another 
>>>> context.
>>>>
>>>> Doesn't QMI provide an equivalent of +CGEV?
>>>
>>> I haven't found an equivalent.  Right now it looks to me as though the
>>> only way to set up a PDP context is to call the "Start Networking"
>>> method.  If I don't start networking, the packet status callback never
>>> gives me a "connected" status.  It's not enough to just power on the
>>> modem... although that does get me registered to the network.
>>
>> QMI is an ancient protocol, so maybe it simply didn't get extended 
>> for LTE.  Or we just don't know how to drive it properly.  Is there 
>> an AT channel ?  Does that one report anything related to CGEV?
>>
>>>
>>> Furthermore, it doesn't matter what APN I use, I always get a usable
>>> data-connection.  "Get Settings" never returns an APN string, either,
>>> which makes me that the network is ignoring my setting altogether and
>>> just giving me a default bearer...???
>>
>> The network is ultimately responsible for allocating an APN.  It 
>> could be blank or null or whatever.  Or maybe we don't know the right 
>> magic incantation.
>
> Alright, I'm going to explain my understanding of how this works:
>
> When the QMI modem registers to an LTE network, it negotiates a 
> default bearer using the "default settings".  The default settings are 
> taken from the QMI WDS "default profile" which is presumably profile 1 
> by default.
>
> Even though the network registration has taken place and the bearer is 
> in place, the QMI modem doesn't bring up the network interface. 
> Calling "get current settings" at this point gives you nothing.
>
> At this point, you need to bring up the network interface by calling 
> "Start network".  Start network takes a bunch of parameters but these 
> parameters are all ignored since the bearer is already set up.
>
> I haven't tested this, but I suspect that calling "Start network" for 
> the same profile again will fail...???  A profile number corresponds 
> to a Context ID...???  Maybe...

I tested this and "start network" fails when attempting to connect a 
profile a second time.

>
> If you need a specific setting for the default bearer (like APN), you 
> need to set it in the default profile before registering to the 
> network.  This is what the LTE atom must take care of... and, hence, 
> the atom must be created
> before setting the modem online.
>
> For non-LTE, the bearer isn't set up until "Start network" is called, 
> so the usual context parameters can be passed to it at that time.  Any 
> parameters that are omitted are taken from the default profile.
>
> Given all of the above, for QMI this means:
>
> i)  When the netreg atom detects registration to an LTE network then 
> we can call "Start network" immediately.
> ii)  Start network will give us the packet-handle for the default 
> bearer.  This can be held by the netreg atom.
> iii)  The packet handle survives until the network becomes 
> unregistered (even through a transition to UMTS,GPRS,etc networks).
> iv)  There is never any reason to call "Stop network" for the default 
> bearer because a network unregistration will achieve the same thing.  
> If the network becomes unregistered we can throw away the packet 
> handle.  The gprs-context associated with the default bearer won't 
> know the packet-handle, so we need to make sure it doesn't try to call 
> "Stop network" for it's instance... but that's a detail, I think.
Point iv) is wrong... we need to release the packet handle using "stop 
network" when the bearer goes away.

/Jonas

> It seems that there exists a method to change the default profile 
> number... it's undocumented, though, AFAICT.
>
> What's a bit unclear to me, though, is how this interacts with network 
> technology transitions... what happens when I move out of LTE coverage 
> and into a UMTS/GPRS network?
>
> Questions?  Feedback appreciated...
> /Jonas
>


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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-12 21:10           ` Jonas Bonn
@ 2017-04-13 15:14             ` Denis Kenzior
  2017-04-14 21:53               ` Jonas Bonn
  0 siblings, 1 reply; 22+ messages in thread
From: Denis Kenzior @ 2017-04-13 15:14 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

>> Alright, I'm going to explain my understanding of how this works:
>>
>> When the QMI modem registers to an LTE network, it negotiates a
>> default bearer using the "default settings".  The default settings are
>> taken from the QMI WDS "default profile" which is presumably profile 1
>> by default.
>>
>> Even though the network registration has taken place and the bearer is
>> in place, the QMI modem doesn't bring up the network interface.
>> Calling "get current settings" at this point gives you nothing.
>>
>> At this point, you need to bring up the network interface by calling
>> "Start network".  Start network takes a bunch of parameters but these
>> parameters are all ignored since the bearer is already set up.
>>

Sounds like some 'creative thinking' from Qualcomm guys ;)  But so far 
this makes sense.  The parameters are already set in stone.  For QMI, 
the .read_settings method should be also bringing up the network 
interface, e.g. 'Start network' in your case.  For ublox devices the 
network device flows packets as soon as the context is active...

>> I haven't tested this, but I suspect that calling "Start network" for
>> the same profile again will fail...???  A profile number corresponds
>> to a Context ID...???  Maybe...
>

I would imagine that a profile has the same meaning as the 27.007 
equivalent of +CGDCONT <cid>

> I tested this and "start network" fails when attempting to connect a
> profile a second time.
>

Makes sense.  Do note that the same APN can be (in theory) double, 
triple, etc activated.  But this requires a separate cid for each instance.

>>
>> If you need a specific setting for the default bearer (like APN), you
>> need to set it in the default profile before registering to the
>> network.  This is what the LTE atom must take care of... and, hence,
>> the atom must be created
>> before setting the modem online.

Correct.  This is what .set_default_attach_info does.  lte atom should 
be created in post_sim.

>>
>> For non-LTE, the bearer isn't set up until "Start network" is called,
>> so the usual context parameters can be passed to it at that time.  Any
>> parameters that are omitted are taken from the default profile.
>>

Again, sounds right.

>> Given all of the above, for QMI this means:
>>
>> i)  When the netreg atom detects registration to an LTE network then
>> we can call "Start network" immediately.

gprs atom driver needs to tell ofono core that a context is active. 
This means gprs driver should call ofono_gprs_cid_activated

>> ii)  Start network will give us the packet-handle for the default
>> bearer.  This can be held by the netreg atom.

why netreg atom?  This should be managed by gprs-context.

>> iii)  The packet handle survives until the network becomes
>> unregistered (even through a transition to UMTS,GPRS,etc networks).
>> iv)  There is never any reason to call "Stop network" for the default
>> bearer because a network unregistration will achieve the same thing.
>> If the network becomes unregistered we can throw away the packet
>> handle.  The gprs-context associated with the default bearer won't
>> know the packet-handle, so we need to make sure it doesn't try to call
>> "Stop network" for it's instance... but that's a detail, I think.
> Point iv) is wrong... we need to release the packet handle using "stop
> network" when the bearer goes away.

Much of this has to be handled by the driver.  E.g. the driver has to 
detect that a default bearer has been activated.  Then:
1. call ofono_gprs_cid_activated
2. Step 1 in turn assigns a context and calls .read_settings
3. .read_settings should setup the active context and call 'start network'
4. When a context is de-activated for whatever reason, gprs-context 
driver needs to clean up, e.g. call 'stop network'.
>
> /Jonas
>
>> It seems that there exists a method to change the default profile
>> number... it's undocumented, though, AFAICT.
>>
>> What's a bit unclear to me, though, is how this interacts with network
>> technology transitions... what happens when I move out of LTE coverage
>> and into a UMTS/GPRS network?

The context should survive and keep going.  If it doesn't, then the 
network / model should tell us that a context is no longer active. 
There must be some notification to that effect.

Regards,
-Denis

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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-13 15:14             ` Denis Kenzior
@ 2017-04-14 21:53               ` Jonas Bonn
  2017-04-14 23:18                 ` Denis Kenzior
  0 siblings, 1 reply; 22+ messages in thread
From: Jonas Bonn @ 2017-04-14 21:53 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

On 04/13/2017 05:14 PM, Denis Kenzior wrote:
> I haven't tested this, but I suspect that calling "Start network" for
>>> the same profile again will fail...??? A profile number corresponds
>>> to a Context ID...???  Maybe...
>>
>
> I would imagine that a profile has the same meaning as the 27.007 
> equivalent of +CGDCONT <cid>

Yes, it does.  Others have tested modifying context parameters with AT 
commands and the changes are visible in the corresponding QMI profile.  
However, this could theoretically be firmware dependent as it's not 
actually specified anywhere that it must be so.

>
>> I tested this and "start network" fails when attempting to connect a
>> profile a second time.
>>
>
> Makes sense.  Do note that the same APN can be (in theory) double, 
> triple, etc activated.  But this requires a separate cid for each 
> instance.

Yes, exactly.  Multiple profiles with identical parameters can be 
activated, as far as I can tell.
>>> Given all of the above, for QMI this means:
>>>
>>> i)  When the netreg atom detects registration to an LTE network then
>>> we can call "Start network" immediately.
>
> gprs atom driver needs to tell ofono core that a context is active. 
> This means gprs driver should call ofono_gprs_cid_activated
>
>>> ii)  Start network will give us the packet-handle for the default
>>> bearer.  This can be held by the netreg atom.
>
> why netreg atom?  This should be managed by gprs-context.

Yes, you're right.  I also came to this insight after writing this email! :)

I originally implemented this as described above but have now posted a 
new series that works as you described.  It really hasn't been easy to 
understand how this all was supposed to work but what I've got now seems 
reasonable and functions well.

One question:  ofono_gprs_notify_bearer()... why do we need this when 
netreg already provides the technology to the GPRS atom?

Thanks for taking the time to help me work through all of this! Much 
appreciated.

/Jonas

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

* Re: [PATCH 5/8] lte: activate default bearer on network registration
  2017-04-14 21:53               ` Jonas Bonn
@ 2017-04-14 23:18                 ` Denis Kenzior
  0 siblings, 0 replies; 22+ messages in thread
From: Denis Kenzior @ 2017-04-14 23:18 UTC (permalink / raw)
  To: ofono

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

Hi Jonas,

>
> I originally implemented this as described above but have now posted a
> new series that works as you described.  It really hasn't been easy to
> understand how this all was supposed to work but what I've got now seems
> reasonable and functions well.

This is one of the most complex pieces of the stack.  It makes my brain 
hurt every time I look at it as well.

>
> One question:  ofono_gprs_notify_bearer()... why do we need this when
> netreg already provides the technology to the GPRS atom?

Many devices send out +CREG/CGREG which only reports status as 'umts'. 
It never elaborates whether the connection is utilizing  plain UMTS, 
HSUPA, HSDPA or combined HSUPA/HSDPA.  Since many carriers made a big 
deal (and special icons) for notifying the user that they're on a 'fast' 
network, we needed a convenient way of grabbing that info.

Manufacturers came up with a bunch of proprietary indications for this 
and eventually 3GPP standardized this inside 27.007 with +CPSB command. 
If you look at drivers/atmodem/gprs.c you can see some of the variants.

>
> Thanks for taking the time to help me work through all of this! Much
> appreciated.
>

Sure, thanks for doing all the work :)

Regards,
-Denis


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

end of thread, other threads:[~2017-04-14 23:18 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11  8:18 [PATCH lte 0/8] QMI LTE series Jonas Bonn
2017-04-11  8:18 ` [PATCH 1/8] qmi: NAS definitions adjustment Jonas Bonn
2017-04-11 15:23   ` Denis Kenzior
2017-04-11  8:18 ` [PATCH 2/8] qmi: add WDS parameter definition Jonas Bonn
2017-04-11 16:23   ` Denis Kenzior
2017-04-11  8:18 ` [PATCH 3/8] qmi: retrieve GPRS context parameters Jonas Bonn
2017-04-11 16:44   ` Denis Kenzior
2017-04-11  8:18 ` [PATCH 4/8] qmi: implement read_settings for automatic contexts Jonas Bonn
2017-04-11 17:02   ` Denis Kenzior
2017-04-11  8:18 ` [PATCH 5/8] lte: activate default bearer on network registration Jonas Bonn
2017-04-11 17:07   ` Denis Kenzior
2017-04-11 20:48     ` Jonas Bonn
2017-04-11 21:21       ` Denis Kenzior
2017-04-12 14:28         ` Jonas Bonn
2017-04-12 21:10           ` Jonas Bonn
2017-04-13 15:14             ` Denis Kenzior
2017-04-14 21:53               ` Jonas Bonn
2017-04-14 23:18                 ` Denis Kenzior
2017-04-11  8:18 ` [PATCH 6/8] qmi: add LTE atom driver Jonas Bonn
2017-04-11 17:09   ` Denis Kenzior
2017-04-11  8:18 ` [PATCH 7/8] qmi: watch for automatic context activation Jonas Bonn
2017-04-11  8:18 ` [PATCH 8/8] gobi: add LTE atom Jonas Bonn

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.