[PATCH 00/14] ublox patches

[PATCH 00/14] ublox patches

[Jonas Bonn]

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

This patch series provides cleanups that should facilitate extending the
ublox driver to more easily cover all device models.  Points of
interest:

- provides model descriptors that can be extended to account for
differences between devices
- queries the device model directly instead of relying on
information from udev
- provides detection and rudimentary support for TOBY L4 modems
- updated LTE driver to be used across all ublox models

The TOBY L4 support is a work in progress and this is also to be extended to
the SARA-R4/N4 series.  I currently only have a TOBY L4 available, so
I'm hoping that somebody with an L2 could give this a shake to make sure
nothing's been broken there.

/Jonas

Jonas Bonn (14):
  Fix whitespace weirdness
  ublox: drop vendor type for Toby L2 model
  ublox: make device selection more flexible
  ublox: create model data structures
  ublox: query device model
  ublox: add model flag HAVE_USBCONF
  ublox: query USBCONF for applicable devices
  udevng: ublox: Model string no longer used
  ublox: add device flags
  ublox: add TOBY L4 models
  udevng: detect ublox TOBY L4
  Separate ATE and AT+CMEE commands
  ublox: extend LTE driver
  ublox: pass model id to LTE plugin

 drivers/atmodem/gprs.c                 |   1 -
 drivers/atmodem/network-registration.c |  19 +--
 drivers/atmodem/sim.c                  |   1 -
 drivers/atmodem/vendor.h               |   1 -
 drivers/ubloxmodem/lte.c               | 101 ++++++++++++--
 drivers/ubloxmodem/ubloxmodem.c        |  78 +++++++++++
 drivers/ubloxmodem/ubloxmodem.h        |  17 +++
 plugins/ublox.c                        | 180 ++++++++++++++++++-------
 plugins/udevng.c                       |  15 ++-
 9 files changed, 336 insertions(+), 77 deletions(-)

-- 
2.19.1

[PATCH 01/14] Fix whitespace weirdness

[Jonas Bonn]

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

---
 drivers/atmodem/network-registration.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 2482dfc3..1704f98c 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -1559,12 +1559,12 @@ static void creg_notify(GAtResult *result, gpointer user_data)
 					option_query_tech_cb, tq, g_free) > 0)
 			return;
 		break;
-    case OFONO_VENDOR_GEMALTO:
-              if (g_at_chat_send(nd->chat, "AT^SMONI",
-                                      smoni_prefix,
-                                      gemalto_query_tech_cb, tq, g_free) > 0)
-                      return;
-              break;
+	case OFONO_VENDOR_GEMALTO:
+		if (g_at_chat_send(nd->chat, "AT^SMONI",
+			      smoni_prefix,
+			      gemalto_query_tech_cb, tq, g_free) > 0)
+			return;
+		break;
 	}
 
 	g_free(tq);
-- 
2.19.1

[PATCH 02/14] ublox: drop vendor type for Toby L2 model

[Jonas Bonn]

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

The 'Toby L2'-specific vendor type is used only to prevent the CMER
command from attempting to set mode '3'.  For all u-blox models, mode
'3' is just an alias for mode '1'.  As such, mode '1' can be used in
place of mode '3', removing the model-specific quirk in favour of a
vendor-specific quirk.

With this change, the L2-specific vendor type is no longer needed.
---
 drivers/atmodem/gprs.c                 | 1 -
 drivers/atmodem/network-registration.c | 7 +++++--
 drivers/atmodem/sim.c                  | 1 -
 drivers/atmodem/vendor.h               | 1 -
 plugins/ublox.c                        | 7 +++----
 5 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index de7c7864..d3fd893c 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -455,7 +455,6 @@ static void gprs_initialized(gboolean ok, GAtResult *result, gpointer user_data)
 						FALSE, gprs, NULL);
 		break;
 	case OFONO_VENDOR_UBLOX:
-	case OFONO_VENDOR_UBLOX_TOBY_L2:
 		g_at_chat_register(gd->chat, "+UREG:", ublox_ureg_notify,
 						FALSE, gprs, NULL);
 		g_at_chat_send(gd->chat, "AT+UREG=1", none_prefix,
diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 1704f98c..ffd8aa33 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -1655,8 +1655,11 @@ static ofono_bool_t build_cmer_string(char *buf, int *cmer_opts,
 	DBG("");
 
 	switch (nd->vendor) {
-	case OFONO_VENDOR_UBLOX_TOBY_L2:
-		/* UBX-13002752 R33: TOBY L2 doesn't support mode 2 and 3 */
+	case OFONO_VENDOR_UBLOX:
+		/* For all u-blox models, mode 3 is equivalent to mode 1;
+		 * since some models do not support setting modes 2 nor 3
+		 * (see UBX-13002752), we prefer mode 1 for all models.
+		 */
 		mode = "1";
 		break;
 	default:
diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 51f84818..5f66a09c 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -1221,7 +1221,6 @@ static void at_pin_retries_query(struct ofono_sim *sim,
 			return;
 		break;
 	case OFONO_VENDOR_UBLOX:
-	case OFONO_VENDOR_UBLOX_TOBY_L2:
 		if (g_at_chat_send(sd->chat, "AT+UPINCNT", upincnt_prefix,
 					upincnt_cb, cbd, g_free) > 0)
 			return;
diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
index cd70c283..10c04315 100644
--- a/drivers/atmodem/vendor.h
+++ b/drivers/atmodem/vendor.h
@@ -46,7 +46,6 @@ enum ofono_vendor {
 	OFONO_VENDOR_QUECTEL,
 	OFONO_VENDOR_QUECTEL_M95,
 	OFONO_VENDOR_UBLOX,
-	OFONO_VENDOR_UBLOX_TOBY_L2,
 	OFONO_VENDOR_XMM,
 	OFONO_VENDOR_GEMALTO,
 };
diff --git a/plugins/ublox.c b/plugins/ublox.c
index 2b3fc5eb..4b9d0410 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -169,13 +169,12 @@ static int ublox_enable(struct ofono_modem *modem)
 	 */
 	data->model_id = atoi(model_str);
 
+	data->vendor_family = OFONO_VENDOR_UBLOX;
+
 	switch (data->model_id) {
 	case SARA_G270:
-		data->vendor_family = OFONO_VENDOR_UBLOX;
-		break;
 	case TOBYL2_COMPATIBLE_MODE:
 	case TOBYL2_HIGH_THROUGHPUT_MODE:
-		data->vendor_family = OFONO_VENDOR_UBLOX_TOBY_L2;
 		break;
 	case TOBYL2_MEDIUM_THROUGHPUT_MODE:
 		DBG("low/medium throughtput profile unsupported");
@@ -189,7 +188,7 @@ static int ublox_enable(struct ofono_modem *modem)
 	if (data->aux == NULL)
 		return -EINVAL;
 
-	if (data->vendor_family == OFONO_VENDOR_UBLOX) {
+	if (data->model_id == SARA_G270) {
 		data->modem = open_device(modem, "Modem", "Modem: ");
 		if (data->modem == NULL) {
 			g_at_chat_unref(data->aux);
-- 
2.19.1

[PATCH 03/14] ublox: make device selection more flexible

[Jonas Bonn]

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

Many ublox modems can sit on either the USB bus or talk directly to a
UART.  The udev plugin mostly takes care of figuring out what ports to
talk to and the protocol is common for all devices after that.

This patch simplifies the setup a bit:
i)  There must always be an aux channel for communication with the modem
ii)  The aux channel may be found behind the string Aux for USB modems
or Device for serial modems
iii)  If the Modem string is set, use it; if not set, assume it's not
available.
---
 plugins/ublox.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index 4b9d0410..32cf1e61 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -185,19 +185,18 @@ static int ublox_enable(struct ofono_modem *modem)
 	}
 
 	data->aux = open_device(modem, "Aux", "Aux: ");
-	if (data->aux == NULL)
-		return -EINVAL;
-
-	if (data->model_id == SARA_G270) {
-		data->modem = open_device(modem, "Modem", "Modem: ");
-		if (data->modem == NULL) {
-			g_at_chat_unref(data->aux);
-			data->aux = NULL;
-			return -EIO;
-		}
+	/* If this is a serial modem then the device may be behind
+	 * the 'Device' attribute instead...
+	 */
+	if (data->aux == NULL) {
+		data->aux = open_device(modem, "Device", "Aux: ");
+		if (data->aux == NULL)
+			return -EINVAL;
+	}
 
+	data->modem = open_device(modem, "Modem", "Modem: ");
+	if (data->modem) {
 		g_at_chat_set_slave(data->modem, data->aux);
-
 		g_at_chat_send(data->modem, "ATE0 +CMEE=1", none_prefix,
 						NULL, NULL, NULL);
 
-- 
2.19.1

[PATCH 04/14] ublox: create model data structures

[Jonas Bonn]

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

This sets up some device model data structures and adds some helpers for
manipulating them.  These data structures are a first step to trying to
support the large number of devices that ublox produces with a common
driver structure.
---
 drivers/ubloxmodem/ubloxmodem.c | 55 +++++++++++++++++++++++++++++++++
 drivers/ubloxmodem/ubloxmodem.h | 14 +++++++++
 2 files changed, 69 insertions(+)

diff --git a/drivers/ubloxmodem/ubloxmodem.c b/drivers/ubloxmodem/ubloxmodem.c
index a325b1f0..682b7cea 100644
--- a/drivers/ubloxmodem/ubloxmodem.c
+++ b/drivers/ubloxmodem/ubloxmodem.c
@@ -33,6 +33,61 @@
 
 #include "ubloxmodem.h"
 
+const struct ublox_model ublox_models[] = {
+	{
+		.name = "SARA-G270",
+	},
+	/* TOBY L2 series */
+	{
+		.name = "TOBY-L200",
+		.flags = UBLOX_F_TOBY_L2,
+	},
+	{
+		.name = "TOBY-L201",
+		.flags = UBLOX_F_TOBY_L2,
+	},
+	{
+		.name = "TOBY-L210",
+		.flags = UBLOX_F_TOBY_L2,
+	},
+	{
+		.name = "TOBY-L220",
+		.flags = UBLOX_F_TOBY_L2,
+	},
+	{
+		.name = "TOBY-L280",
+		.flags = UBLOX_F_TOBY_L2,
+	},
+	{ /* sentinal */ },
+};
+
+const struct ublox_model* ublox_model_from_name(const char* name)
+{
+	const struct ublox_model* m;
+
+	for (m = ublox_models; m->name; m++) {
+		if (!strcmp(name, m->name))
+			return m;
+	}
+
+	return NULL;
+}
+
+const struct ublox_model* ublox_model_from_id(int id)
+{
+	return ublox_models + id;
+}
+
+int ublox_model_to_id(const struct ublox_model* model)
+{
+	return model - ublox_models;
+}
+
+int ublox_is_toby_l2(const struct ublox_model* model)
+{
+	return model->flags & UBLOX_F_TOBY_L2;
+}
+
 static int ubloxmodem_init(void)
 {
 	ublox_gprs_context_init();
diff --git a/drivers/ubloxmodem/ubloxmodem.h b/drivers/ubloxmodem/ubloxmodem.h
index bfb01064..e8f98b40 100644
--- a/drivers/ubloxmodem/ubloxmodem.h
+++ b/drivers/ubloxmodem/ubloxmodem.h
@@ -23,6 +23,20 @@
 
 #define UBLOXMODEM "ubloxmodem"
 
+enum ublox_flags {
+	UBLOX_F_TOBY_L2		= (1 << 0),
+};
+
+struct ublox_model {
+	char* name;
+	int flags;
+};
+
+const struct ublox_model* ublox_model_from_name(const char* name);
+const struct ublox_model* ublox_model_from_id(int id);
+int ublox_model_to_id(const struct ublox_model* model);
+int ublox_is_toby_l2(const struct ublox_model* model);
+
 extern void ublox_gprs_context_init(void);
 extern void ublox_gprs_context_exit(void);
 
-- 
2.19.1

[PATCH 05/14] ublox: query device model

[Jonas Bonn]

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

This patch adds a call to CGMM into the modem_enable path in order to
establish the specific device model.  From this device model string, a
model-specific capabilities structure can be selected.
---
 plugins/ublox.c | 107 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 77 insertions(+), 30 deletions(-)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index 32cf1e61..efb0afbc 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -42,9 +42,10 @@
 #include <ofono/lte.h>
 #include <ofono/voicecall.h>
 
-#include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
+#include <drivers/ubloxmodem/ubloxmodem.h>
+
 static const char *none_prefix[] = { NULL };
 
 enum supported_models {
@@ -59,6 +60,8 @@ struct ublox_data {
 	GAtChat *aux;
 	int model_id;
 	enum ofono_vendor vendor_family;
+
+	const struct ublox_model* model;
 };
 
 static void ublox_debug(const char *str, void *user_data)
@@ -152,38 +155,79 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_modem_set_powered(modem, TRUE);
 }
 
-static int ublox_enable(struct ofono_modem *modem)
+static void query_model_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
+	struct ofono_modem* modem = user_data;
 	struct ublox_data *data = ofono_modem_get_data(modem);
-	const char *model_str = NULL;
+	struct ofono_error error;
+	const char *model;
+	const struct ublox_model* m;
+	const char* model_str;
 
-	DBG("%p", modem);
+	decode_at_error(&error, g_at_result_final_response(result));
 
-	model_str = ofono_modem_get_string(modem, "Model");
-	if (model_str == NULL)
-		return -EINVAL;
+	if (!ok)
+		goto fail;
 
-	/*
-	 * Toby L2 devices are more complex and special than previously
-	 * supported U-Blox devices. So they need a vendor of their own.
-	 */
-	data->model_id = atoi(model_str);
-
-	data->vendor_family = OFONO_VENDOR_UBLOX;
-
-	switch (data->model_id) {
-	case SARA_G270:
-	case TOBYL2_COMPATIBLE_MODE:
-	case TOBYL2_HIGH_THROUGHPUT_MODE:
-		break;
-	case TOBYL2_MEDIUM_THROUGHPUT_MODE:
-		DBG("low/medium throughtput profile unsupported");
-		break;
-	default:
-		DBG("unknown ublox model id %d", data->model_id);
-		return -EINVAL;
+	if (at_util_parse_attr(result, "", &model) == FALSE) {
+		ofono_error("Failed to query modem model");
+		goto fail;
 	}
 
+	m = ublox_model_from_name(model);
+	if (!m->name) {
+		ofono_error("Unrecognized model: %s", model);
+		goto fail;
+	}
+
+	data->model = m;
+
+	DBG("Model: %s", data->model->name);
+
+	if (data->model->flags & UBLOX_F_TOBY_L2) {
+		model_str = ofono_modem_get_string(modem, "Model");
+		if (!model_str)
+			goto fail;
+
+		/*
+		 * Toby L2 devices are more complex and special than previously
+		 * supported U-Blox devices. So they need a vendor of their own.
+		 */
+		data->model_id = atoi(model_str);
+
+		switch (data->model_id) {
+		case TOBYL2_COMPATIBLE_MODE:
+		case TOBYL2_HIGH_THROUGHPUT_MODE:
+			break;
+		case TOBYL2_MEDIUM_THROUGHPUT_MODE:
+			DBG("low/medium throughtput profile unsupported");
+			break;
+		default:
+			DBG("unknown ublox model id %d", data->model_id);
+			goto fail;
+		}
+	} else {
+		data->vendor_family = OFONO_VENDOR_UBLOX;
+	}
+
+	if (g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix,
+					cfun_enable, modem, NULL))
+		return;
+
+fail:
+	g_at_chat_unref(data->aux);
+	data->aux = NULL;
+	g_at_chat_unref(data->modem);
+	data->modem = NULL;
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+static int ublox_enable(struct ofono_modem *modem)
+{
+	struct ublox_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
 	data->aux = open_device(modem, "Aux", "Aux: ");
 	/* If this is a serial modem then the device may be behind
 	 * the 'Device' attribute instead...
@@ -199,7 +243,6 @@ static int ublox_enable(struct ofono_modem *modem)
 		g_at_chat_set_slave(data->modem, data->aux);
 		g_at_chat_send(data->modem, "ATE0 +CMEE=1", none_prefix,
 						NULL, NULL, NULL);
-
 		g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL);
 	}
 
@@ -209,10 +252,14 @@ static int ublox_enable(struct ofono_modem *modem)
 	g_at_chat_send(data->aux, "ATE0 +CMEE=1", none_prefix,
 					NULL, NULL, NULL);
 
-	g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix,
-					cfun_enable, modem, NULL);
+	if (g_at_chat_send(data->aux, "AT+CGMM", NULL,
+				query_model_cb, modem, NULL) > 0)
+		return -EINPROGRESS;
 
-	return -EINPROGRESS;
+	g_at_chat_unref(data->aux);
+	data->aux = NULL;
+
+	return -EINVAL;
 }
 
 static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
-- 
2.19.1

[PATCH 06/14] ublox: add model flag HAVE_USBCONF

[Jonas Bonn]

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

This flag should be set on devices that support multiple USB profiles
and will be used to determine whether or not querying USB profiles is
appropriate/necessary.
---
 drivers/ubloxmodem/ubloxmodem.c | 10 +++++-----
 drivers/ubloxmodem/ubloxmodem.h |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/ubloxmodem/ubloxmodem.c b/drivers/ubloxmodem/ubloxmodem.c
index 682b7cea..3998f55e 100644
--- a/drivers/ubloxmodem/ubloxmodem.c
+++ b/drivers/ubloxmodem/ubloxmodem.c
@@ -40,23 +40,23 @@ const struct ublox_model ublox_models[] = {
 	/* TOBY L2 series */
 	{
 		.name = "TOBY-L200",
-		.flags = UBLOX_F_TOBY_L2,
+		.flags = UBLOX_F_TOBY_L2|UBLOX_F_HAVE_USBCONF,
 	},
 	{
 		.name = "TOBY-L201",
-		.flags = UBLOX_F_TOBY_L2,
+		.flags = UBLOX_F_TOBY_L2|UBLOX_F_HAVE_USBCONF,
 	},
 	{
 		.name = "TOBY-L210",
-		.flags = UBLOX_F_TOBY_L2,
+		.flags = UBLOX_F_TOBY_L2|UBLOX_F_HAVE_USBCONF,
 	},
 	{
 		.name = "TOBY-L220",
-		.flags = UBLOX_F_TOBY_L2,
+		.flags = UBLOX_F_TOBY_L2|UBLOX_F_HAVE_USBCONF,
 	},
 	{
 		.name = "TOBY-L280",
-		.flags = UBLOX_F_TOBY_L2,
+		.flags = UBLOX_F_TOBY_L2|UBLOX_F_HAVE_USBCONF,
 	},
 	{ /* sentinal */ },
 };
diff --git a/drivers/ubloxmodem/ubloxmodem.h b/drivers/ubloxmodem/ubloxmodem.h
index e8f98b40..90749bff 100644
--- a/drivers/ubloxmodem/ubloxmodem.h
+++ b/drivers/ubloxmodem/ubloxmodem.h
@@ -25,6 +25,7 @@
 
 enum ublox_flags {
 	UBLOX_F_TOBY_L2		= (1 << 0),
+	UBLOX_F_HAVE_USBCONF	= (1 << 2),
 };
 
 struct ublox_model {
-- 
2.19.1

[PATCH 07/14] ublox: query USBCONF for applicable devices

[Jonas Bonn]

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

The TOBY L2 series of modems presents a number of different
configurations with different throughtput characteristics.  These
configurations are packaged up as USB profiles; moreover, changing the
profile actually changes the USB model ID so this is even more like
selecting a different "device" altogether.  Nonetheless, all we need to
know is which profile is selected in order to set things up correctly
and this can be queried directly.

This patch adds a call to UUSBCONF for applicable modems in order to
query the USB configuration to find out which profile is active.
---
 plugins/ublox.c | 82 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 23 deletions(-)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index efb0afbc..f7c2a374 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -46,6 +46,7 @@
 
 #include <drivers/ubloxmodem/ubloxmodem.h>
 
+static const char *uusbconf_prefix[] = { "+UUSBCONF:", NULL };
 static const char *none_prefix[] = { NULL };
 
 enum supported_models {
@@ -155,6 +156,57 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_modem_set_powered(modem, TRUE);
 }
 
+static void query_usbconf_cb(gboolean ok, GAtResult *result, gpointer user_data){
+	struct ofono_modem* modem = user_data;
+	struct ublox_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int profile;
+
+	if (!ok) {
+		ofono_error("Unable to query USB configuration");
+		goto error;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+retry:
+	if (!g_at_result_iter_next(&iter, "+UUSBCONF")) {
+		ofono_error("Unable to query USB configuration");
+		goto error;
+	}
+
+	if (!g_at_result_iter_next_number(&iter, &profile))
+		goto retry;
+
+	switch (profile) {
+	case 0: /* Fairly back compatible */
+	case 1: /* Fairly back compatible plus audio */
+		data->model_id =  TOBYL2_COMPATIBLE_MODE;
+		break;
+	case 2: /* Low/medium throughput */
+		data->model_id = TOBYL2_MEDIUM_THROUGHPUT_MODE;
+		break;
+	case 3: /* High throughput mode */
+		data->model_id = TOBYL2_HIGH_THROUGHPUT_MODE;
+		break;
+	default:
+		ofono_error("Unexpected USB profile: %d", profile);
+		goto error;
+	}
+
+	if (g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix,
+					cfun_enable, modem, NULL))
+		return;
+
+error:
+	g_at_chat_unref(data->aux);
+	data->aux = NULL;
+	g_at_chat_unref(data->modem);
+	data->modem = NULL;
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+
 static void query_model_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem* modem = user_data;
@@ -162,7 +214,6 @@ static void query_model_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_error error;
 	const char *model;
 	const struct ublox_model* m;
-	const char* model_str;
 
 	decode_at_error(&error, g_at_result_final_response(result));
 
@@ -184,28 +235,13 @@ static void query_model_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	DBG("Model: %s", data->model->name);
 
-	if (data->model->flags & UBLOX_F_TOBY_L2) {
-		model_str = ofono_modem_get_string(modem, "Model");
-		if (!model_str)
-			goto fail;
-
-		/*
-		 * Toby L2 devices are more complex and special than previously
-		 * supported U-Blox devices. So they need a vendor of their own.
-		 */
-		data->model_id = atoi(model_str);
-
-		switch (data->model_id) {
-		case TOBYL2_COMPATIBLE_MODE:
-		case TOBYL2_HIGH_THROUGHPUT_MODE:
-			break;
-		case TOBYL2_MEDIUM_THROUGHPUT_MODE:
-			DBG("low/medium throughtput profile unsupported");
-			break;
-		default:
-			DBG("unknown ublox model id %d", data->model_id);
-			goto fail;
-		}
+	if (data->model->flags & UBLOX_F_HAVE_USBCONF) {
+		if (g_at_chat_send(data->aux, "AT+UUSBCONF?", uusbconf_prefix,
+					query_usbconf_cb, modem, NULL))
+			return;
+
+		ofono_error("Unable to query USB configuration");
+		goto fail;
 	} else {
 		data->vendor_family = OFONO_VENDOR_UBLOX;
 	}
-- 
2.19.1

[PATCH 08/14] udevng: ublox: Model string no longer used

[Jonas Bonn]

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

---
 plugins/udevng.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index ce8cdee1..4473c0a6 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1111,7 +1111,6 @@ static gboolean setup_ublox(struct modem_info *modem)
 
 	ofono_modem_set_string(modem->modem, "Aux", aux);
 	ofono_modem_set_string(modem->modem, "Modem", mdm);
-	ofono_modem_set_string(modem->modem, "Model", modem->model);
 	ofono_modem_set_string(modem->modem, "NetworkInterface", net);
 
 	return TRUE;
-- 
2.19.1

[PATCH 09/14] ublox: add device flags

[Jonas Bonn]

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

Some aspects of a device are detectable at runtime, like the USB profile
detection that was added in a patch preceding this one.  This patch
switches the driver over from creating a new "vendor id" for each
profile to just setting a flag.  This is more easily extensible as we
detect other features of the modem.
---
 plugins/ublox.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index f7c2a374..038b55d5 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -49,20 +49,17 @@
 static const char *uusbconf_prefix[] = { "+UUSBCONF:", NULL };
 static const char *none_prefix[] = { NULL };
 
-enum supported_models {
-	SARA_G270			= 1102,
-	TOBYL2_COMPATIBLE_MODE		= 1141,
-	TOBYL2_MEDIUM_THROUGHPUT_MODE	= 1143,
-	TOBYL2_HIGH_THROUGHPUT_MODE	= 1146,
+enum ublox_device_flags {
+	UBLOX_DEVICE_F_HIGH_THROUGHPUT_MODE	= (1 << 0),
 };
 
 struct ublox_data {
 	GAtChat *modem;
 	GAtChat *aux;
-	int model_id;
 	enum ofono_vendor vendor_family;
 
 	const struct ublox_model* model;
+	int flags;
 };
 
 static void ublox_debug(const char *str, void *user_data)
@@ -148,7 +145,7 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	if (data->model_id == TOBYL2_HIGH_THROUGHPUT_MODE)
+	if (data->flags & UBLOX_DEVICE_F_HIGH_THROUGHPUT_MODE)
 		/* use bridged mode until routed mode support is added */
 		g_at_chat_send(data->aux, "AT+UBMCONF=2", none_prefix,
 						NULL, NULL, NULL);
@@ -181,13 +178,12 @@ retry:
 	switch (profile) {
 	case 0: /* Fairly back compatible */
 	case 1: /* Fairly back compatible plus audio */
-		data->model_id =  TOBYL2_COMPATIBLE_MODE;
 		break;
 	case 2: /* Low/medium throughput */
-		data->model_id = TOBYL2_MEDIUM_THROUGHPUT_MODE;
-		break;
+		ofono_error("Medium throughput mode not supported");
+		goto error;
 	case 3: /* High throughput mode */
-		data->model_id = TOBYL2_HIGH_THROUGHPUT_MODE;
+		data->flags |= UBLOX_DEVICE_F_HIGH_THROUGHPUT_MODE;
 		break;
 	default:
 		ofono_error("Unexpected USB profile: %d", profile);
@@ -389,10 +385,10 @@ static void ublox_post_sim(struct ofono_modem *modem)
 	struct ofono_gprs *gprs;
 	struct ofono_gprs_context *gc;
 	GAtChat *chat = data->modem ? data->modem : data->aux;
-	const char *driver = data->model_id == TOBYL2_HIGH_THROUGHPUT_MODE ?
+	const char *driver = data->flags & UBLOX_DEVICE_F_HIGH_THROUGHPUT_MODE ?
 						"ubloxmodem" : "atmodem";
 	/* Toby L2: Create same number of contexts as supported PDP contexts. */
-	int ncontexts = data->model_id == TOBYL2_HIGH_THROUGHPUT_MODE ? 8 : 1;
+	int ncontexts = data->flags & UBLOX_DEVICE_F_HIGH_THROUGHPUT_MODE ? 8 : 1;
 
 	DBG("%p", modem);
 
-- 
2.19.1

[PATCH 10/14] ublox: add TOBY L4 models

[Jonas Bonn]

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

---
 drivers/ubloxmodem/ubloxmodem.c | 23 +++++++++++++++++++++++
 drivers/ubloxmodem/ubloxmodem.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/ubloxmodem/ubloxmodem.c b/drivers/ubloxmodem/ubloxmodem.c
index 3998f55e..49a824ed 100644
--- a/drivers/ubloxmodem/ubloxmodem.c
+++ b/drivers/ubloxmodem/ubloxmodem.c
@@ -58,6 +58,23 @@ const struct ublox_model ublox_models[] = {
 		.name = "TOBY-L280",
 		.flags = UBLOX_F_TOBY_L2|UBLOX_F_HAVE_USBCONF,
 	},
+	/* TOBY L4 series */
+	{
+		.name = "TOBY-L4006",
+		.flags = UBLOX_F_TOBY_L4,
+	},
+	{
+		.name = "TOBY-L4106",
+		.flags = UBLOX_F_TOBY_L4,
+	},
+	{
+		.name = "TOBY-L4206",
+		.flags = UBLOX_F_TOBY_L4,
+	},
+	{
+		.name = "TOBY-L4906",
+		.flags = UBLOX_F_TOBY_L4,
+	},
 	{ /* sentinal */ },
 };
 
@@ -88,6 +105,12 @@ int ublox_is_toby_l2(const struct ublox_model* model)
 	return model->flags & UBLOX_F_TOBY_L2;
 }
 
+int ublox_is_toby_l4(const struct ublox_model* model)
+{
+	return model->flags & UBLOX_F_TOBY_L4;
+}
+
+
 static int ubloxmodem_init(void)
 {
 	ublox_gprs_context_init();
diff --git a/drivers/ubloxmodem/ubloxmodem.h b/drivers/ubloxmodem/ubloxmodem.h
index 90749bff..8a499c11 100644
--- a/drivers/ubloxmodem/ubloxmodem.h
+++ b/drivers/ubloxmodem/ubloxmodem.h
@@ -25,6 +25,7 @@
 
 enum ublox_flags {
 	UBLOX_F_TOBY_L2		= (1 << 0),
+	UBLOX_F_TOBY_L4		= (1 << 1),
 	UBLOX_F_HAVE_USBCONF	= (1 << 2),
 };
 
@@ -37,6 +38,7 @@ const struct ublox_model* ublox_model_from_name(const char* name);
 const struct ublox_model* ublox_model_from_id(int id);
 int ublox_model_to_id(const struct ublox_model* model);
 int ublox_is_toby_l2(const struct ublox_model* model);
+int ublox_is_toby_l4(const struct ublox_model* model);
 
 extern void ublox_gprs_context_init(void);
 extern void ublox_gprs_context_exit(void);
-- 
2.19.1

[PATCH 11/14] udevng: detect ublox TOBY L4

[Jonas Bonn]

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

ttyACM0 (USB interface 02) is reportedly unreliable (breaking DHCP setup)
so the recommended approach is to use ttyACM2 (USB interface 06)
exclusively.
---
 plugins/udevng.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 4473c0a6..f689b756 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1090,11 +1090,17 @@ static gboolean setup_ublox(struct modem_info *modem)
 		 *  - high throughput profile : 224/1/3
 		 */
 		} else if (g_strcmp0(info->interface, "2/2/1") == 0) {
-			if (g_strcmp0(info->number, "02") == 0)
-				aux = info->devnode;
-			else if (g_strcmp0(info->number, "00") == 0)
+			if (!g_strcmp0(modem->model, "1010")) {
+				if (g_strcmp0(info->number, "06") == 0)
+					aux = info->devnode;
+			} else {
+				if (g_strcmp0(info->number, "02") == 0)
+					aux = info->devnode;
+			}
+			if (g_strcmp0(info->number, "00") == 0)
 				mdm = info->devnode;
 		} else if (g_strcmp0(info->interface, "2/6/0") == 0 ||
+				g_strcmp0(info->interface, "2/13/0") == 0 ||
 				g_strcmp0(info->interface, "10/0/0") == 0 ||
 				g_strcmp0(info->interface, "224/1/3") == 0) {
 			net = info->devnode;
@@ -1691,6 +1697,8 @@ static struct {
 	{ "quectelqmi",	"qcserial",	"2c7c", "0121"	},
 	{ "quectelqmi",	"qmi_wwan",	"2c7c", "0125"	},
 	{ "quectelqmi",	"qcserial",	"2c7c", "0125"	},
+	{ "ublox",	"cdc_acm",	"1546", "1010"	},
+	{ "ublox",	"cdc_ncm",	"1546", "1010"	},
 	{ "ublox",	"cdc_acm",	"1546", "1102"	},
 	{ "ublox",	"rndis_host",	"1546", "1146"	},
 	{ "ublox",	"cdc_acm",	"1546", "1146"	},
-- 
2.19.1

[PATCH 12/14] Separate ATE and AT+CMEE commands

[Jonas Bonn]

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

The TOBY L4 doesn't seem to like seeing these two commands on the same
line...
---
 plugins/ublox.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index 038b55d5..7a93adb2 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -281,7 +281,9 @@ static int ublox_enable(struct ofono_modem *modem)
 	/* The modem can take a while to wake up if just powered on. */
 	g_at_chat_set_wakeup_command(data->aux, "AT\r", 1000, 11000);
 
-	g_at_chat_send(data->aux, "ATE0 +CMEE=1", none_prefix,
+	g_at_chat_send(data->aux, "ATE0", none_prefix,
+					NULL, NULL, NULL);
+	g_at_chat_send(data->aux, "AT+CMEE=1", none_prefix,
 					NULL, NULL, NULL);
 
 	if (g_at_chat_send(data->aux, "AT+CGMM", NULL,
-- 
2.19.1

[PATCH 13/14] ublox: extend LTE driver

[Jonas Bonn]

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

There are a couple of semi-independent changes here:

* use the 'vendor' parameter to pass the modem 'model'
* support TOBY L4 modem which uses a fixed CID for configuring the EPS
default bearer
* add the setup of authentication parameters that was recently added to
the atmodem LTE driver
---
 drivers/ubloxmodem/lte.c | 101 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 89 insertions(+), 12 deletions(-)

diff --git a/drivers/ubloxmodem/lte.c b/drivers/ubloxmodem/lte.c
index 34397dfe..5b134745 100644
--- a/drivers/ubloxmodem/lte.c
+++ b/drivers/ubloxmodem/lte.c
@@ -39,24 +39,85 @@
 
 #include "ubloxmodem.h"
 
-static const char *ucgdflt_prefix[] = { "+UCGDFLT:", NULL };
+static const char *none_prefix[] = { NULL };
 
 struct lte_driver_data {
 	GAtChat *chat;
+	const struct ublox_model* model;
+	struct ofono_lte_default_attach_info pending_info;
 };
 
-static void ucgdflt_cb(gboolean ok, GAtResult *result, gpointer user_data)
+static void at_lte_set_auth_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
 	ofono_lte_cb_t cb = cbd->cb;
 	struct ofono_error error;
 
-	DBG("ok %d", ok);
-
 	decode_at_error(&error, g_at_result_final_response(result));
 	cb(&error, cbd->data);
 }
 
+static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_lte_cb_t cb = cbd->cb;
+	void *data = cbd->data;
+	struct lte_driver_data *ldd = cbd->user;
+	struct ofono_error error;
+	char buf[32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+					OFONO_GPRS_MAX_PASSWORD_LENGTH  + 1];
+	enum ofono_gprs_auth_method auth_method;
+	int cid;
+
+	if (!ok) {
+		decode_at_error(&error, g_at_result_final_response(result));
+		cb(&error, data);
+		return;
+	}
+
+	if (ublox_is_toby_l2(ldd->model)) {
+		/* If CGDCONT has already been used to set up cid 4 then
+		 * the EPS default bearer will be configured from another
+		 * cid (see documentation for how this is selected).  Avoid
+		 * doing so as this assumes as much...
+		 */
+		cid = 4;
+	} else if (ublox_is_toby_l4(ldd->model)) {
+		cid = 1;
+	} else {
+		ofono_error("Unknown model; "
+			"unable to determine EPS default bearer CID");
+		goto out;
+	}
+
+	auth_method = ldd->pending_info.auth_method;
+
+	/* change the authentication method if the  parameters are invalid */
+	if (!*ldd->pending_info.username || !*ldd->pending_info.password)
+		auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+	/* In contrast to CGAUTH, all four parameters are _required_ here;
+	 * if auth type is NONE then username and password must be set to
+	 * empty strings.
+	 */
+	sprintf(buf, "AT+UAUTHREQ=%d,%d,\"%s\",\"%s\"",
+			cid,
+			at_util_gprs_auth_method_to_auth_prot(auth_method),
+			ldd->pending_info.username,
+			ldd->pending_info.password);
+
+	cbd = cb_data_ref(cbd);
+	if (g_at_chat_send(ldd->chat, buf, none_prefix,
+			at_lte_set_auth_cb, cbd, cb_data_unref) > 0)
+		return;
+
+out:
+	cb_data_unref(cbd);
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static void ublox_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)
@@ -67,17 +128,32 @@ static void ublox_lte_set_default_attach_info(const struct ofono_lte *lte,
 
 	DBG("LTE config with APN: %s", info->apn);
 
-	if (strlen(info->apn) > 0)
-		snprintf(buf, sizeof(buf), "AT+UCGDFLT=0,\"IP\",\"%s\"",
+	cbd->user = ldd;
+	memcpy(&ldd->pending_info, info, sizeof(ldd->pending_info));
+
+	if (ublox_is_toby_l2(ldd->model)) {
+		if (strlen(info->apn) > 0)
+			snprintf(buf, sizeof(buf), "AT+UCGDFLT=0,%s,\"%s\"",
+				at_util_gprs_proto_to_pdp_type(info->proto),
+				info->apn);
+		else
+			snprintf(buf, sizeof(buf), "AT+UCGDFLT=0");
+
+	} else if (ublox_is_toby_l4(ldd->model)) {
+		if (strlen(info->apn) > 0)
+			snprintf(buf, sizeof(buf), "AT+CGDCONT=1,%s,\"%s\"",
+				at_util_gprs_proto_to_pdp_type(info->proto),
 				info->apn);
-	else
-		snprintf(buf, sizeof(buf), "AT+UCGDFLT=0");
+		else
+			snprintf(buf, sizeof(buf), "AT+CGDCONT=1");
+	}
 
-	/* We can't do much in case of failure so don't check response. */
-	if (g_at_chat_send(ldd->chat, buf, ucgdflt_prefix,
-				ucgdflt_cb, cbd, g_free) > 0)
+	if (g_at_chat_send(ldd->chat, buf, none_prefix,
+			at_lte_set_default_attach_info_cb,
+			cbd, cb_data_unref) > 0)
 		return;
 
+	cb_data_unref(cbd);
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
@@ -91,7 +167,7 @@ static gboolean lte_delayed_register(gpointer user_data)
 }
 
 static int ublox_lte_probe(struct ofono_lte *lte,
-					unsigned int vendor, void *data)
+					unsigned int model_id, void *data)
 {
 	GAtChat *chat = data;
 	struct lte_driver_data *ldd;
@@ -103,6 +179,7 @@ static int ublox_lte_probe(struct ofono_lte *lte,
 		return -ENOMEM;
 
 	ldd->chat = g_at_chat_clone(chat);
+	ldd->model = ublox_model_from_id(model_id);
 
 	ofono_lte_set_data(lte, ldd);
 
-- 
2.19.1

[PATCH 14/14] ublox: pass model id to LTE plugin

[Jonas Bonn]

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

---
 plugins/ublox.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index 7a93adb2..bec0bf83 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -407,7 +407,8 @@ static void ublox_post_sim(struct ofono_modem *modem)
 		--ncontexts;
 	}
 
-	ofono_lte_create(modem, 0, "ubloxmodem", data->aux);
+	ofono_lte_create(modem,
+		ublox_model_to_id(data->model), "ubloxmodem", data->aux);
 }
 
 static void ublox_post_online(struct ofono_modem *modem)
-- 
2.19.1

Re: [PATCH 01/14] Fix whitespace weirdness

[Denis Kenzior]

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

Hi Jonas,

On 03/12/2019 06:09 AM, Jonas Bonn wrote:
> ---
>   drivers/atmodem/network-registration.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
> index 2482dfc3..1704f98c 100644
> --- a/drivers/atmodem/network-registration.c
> +++ b/drivers/atmodem/network-registration.c
> @@ -1559,12 +1559,12 @@ static void creg_notify(GAtResult *result, gpointer user_data)
>   					option_query_tech_cb, tq, g_free) > 0)
>   			return;
>   		break;
> -    case OFONO_VENDOR_GEMALTO:
> -              if (g_at_chat_send(nd->chat, "AT^SMONI",
> -                                      smoni_prefix,
> -                                      gemalto_query_tech_cb, tq, g_free) > 0)
> -                      return;
> -              break;
> +	case OFONO_VENDOR_GEMALTO:
> +		if (g_at_chat_send(nd->chat, "AT^SMONI",
> +			      smoni_prefix,
> +			      gemalto_query_tech_cb, tq, g_free) > 0)

You still left some spaces in here.  Anyway I fixed this up for you and 
applied.

> +			return;
> +		break;
>   	}
>   
>   	g_free(tq);
> 

Regards,
-Denis

Re: [PATCH 02/14] ublox: drop vendor type for Toby L2 model

[Denis Kenzior]

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

Hi Jonas,

On 03/12/2019 06:09 AM, Jonas Bonn wrote:
> The 'Toby L2'-specific vendor type is used only to prevent the CMER
> command from attempting to set mode '3'.  For all u-blox models, mode
> '3' is just an alias for mode '1'.  As such, mode '1' can be used in
> place of mode '3', removing the model-specific quirk in favour of a
> vendor-specific quirk.
> 
> With this change, the L2-specific vendor type is no longer needed.
> ---
>   drivers/atmodem/gprs.c                 | 1 -
>   drivers/atmodem/network-registration.c | 7 +++++--
>   drivers/atmodem/sim.c                  | 1 -
>   drivers/atmodem/vendor.h               | 1 -
>   plugins/ublox.c                        | 7 +++----
>   5 files changed, 8 insertions(+), 9 deletions(-)
> 

I split this up into 3 commits in line with our patch submission guidelines.

Applied, thanks.

Regards,
-Denis

Re: [PATCH 05/14] ublox: query device model

[Denis Kenzior]

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

Hi Jonas,

On 03/12/2019 06:09 AM, Jonas Bonn wrote:
> This patch adds a call to CGMM into the modem_enable path in order to
> establish the specific device model.  From this device model string, a
> model-specific capabilities structure can be selected.
> ---
>   plugins/ublox.c | 107 ++++++++++++++++++++++++++++++++++--------------
>   1 file changed, 77 insertions(+), 30 deletions(-)
> 

Have you tried running checkpatch.pl on these?  There are too many 
coding style violations for me to fix.

Here's a small sample:

ERROR: "foo* bar" should be "foo *bar"
#138: FILE: plugins/ublox.c:64:
+	const struct ublox_model* model;

ERROR: DOS line endings
#147: FILE: plugins/ublox.c:158:
+static void query_model_cb(gboolean ok, GAtResult *result, gpointer 
user_data)^M$

ERROR: DOS line endings
#149: FILE: plugins/ublox.c:160:
+^Istruct ofono_modem* modem = user_data;^M$

ERROR: "foo* bar" should be "foo *bar"
#149: FILE: plugins/ublox.c:160:
+	struct ofono_modem* modem = user_data;



> diff --git a/plugins/ublox.c b/plugins/ublox.c
> index 32cf1e61..efb0afbc 100644
> --- a/plugins/ublox.c
> +++ b/plugins/ublox.c
> @@ -42,9 +42,10 @@
>   #include <ofono/lte.h>
>   #include <ofono/voicecall.h>
>   
> -#include <drivers/atmodem/atutil.h>
>   #include <drivers/atmodem/vendor.h>
>   
> +#include <drivers/ubloxmodem/ubloxmodem.h>
> +
>   static const char *none_prefix[] = { NULL };
>   
>   enum supported_models {
> @@ -59,6 +60,8 @@ struct ublox_data {
>   	GAtChat *aux;
>   	int model_id;
>   	enum ofono_vendor vendor_family;
> +
> +	const struct ublox_model* model;
>   };
>   
>   static void ublox_debug(const char *str, void *user_data)
> @@ -152,38 +155,79 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>   	ofono_modem_set_powered(modem, TRUE);
>   }
>   
> -static int ublox_enable(struct ofono_modem *modem)
> +static void query_model_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   {
> +	struct ofono_modem* modem = user_data;
>   	struct ublox_data *data = ofono_modem_get_data(modem);
> -	const char *model_str = NULL;
> +	struct ofono_error error;
> +	const char *model;
> +	const struct ublox_model* m;
> +	const char* model_str;
>   
> -	DBG("%p", modem);
> +	decode_at_error(&error, g_at_result_final_response(result));
>   
> -	model_str = ofono_modem_get_string(modem, "Model");
> -	if (model_str == NULL)
> -		return -EINVAL;
> +	if (!ok)
> +		goto fail;
>   
> -	/*
> -	 * Toby L2 devices are more complex and special than previously
> -	 * supported U-Blox devices. So they need a vendor of their own.
> -	 */
> -	data->model_id = atoi(model_str);
> -
> -	data->vendor_family = OFONO_VENDOR_UBLOX;
> -
> -	switch (data->model_id) {
> -	case SARA_G270:
> -	case TOBYL2_COMPATIBLE_MODE:
> -	case TOBYL2_HIGH_THROUGHPUT_MODE:
> -		break;
> -	case TOBYL2_MEDIUM_THROUGHPUT_MODE:
> -		DBG("low/medium throughtput profile unsupported");
> -		break;
> -	default:
> -		DBG("unknown ublox model id %d", data->model_id);
> -		return -EINVAL;
> +	if (at_util_parse_attr(result, "", &model) == FALSE) {
> +		ofono_error("Failed to query modem model");
> +		goto fail;
>   	}
>   
> +	m = ublox_model_from_name(model);
> +	if (!m->name) {

So you just potentially crashed here

> +		ofono_error("Unrecognized model: %s", model);
> +		goto fail;
> +	}
> +
> +	data->model = m;
> +
> +	DBG("Model: %s", data->model->name);
> +
> +	if (data->model->flags & UBLOX_F_TOBY_L2) {
> +		model_str = ofono_modem_get_string(modem, "Model");
> +		if (!model_str)
> +			goto fail;
> +
> +		/*
> +		 * Toby L2 devices are more complex and special than previously
> +		 * supported U-Blox devices. So they need a vendor of their own.
> +		 */
> +		data->model_id = atoi(model_str);
> +

atoi is generally considered harmful.  Use strtoul instead

> +		switch (data->model_id) {
> +		case TOBYL2_COMPATIBLE_MODE:
> +		case TOBYL2_HIGH_THROUGHPUT_MODE:
> +			break;
> +		case TOBYL2_MEDIUM_THROUGHPUT_MODE:
> +			DBG("low/medium throughtput profile unsupported");
> +			break;
> +		default:
> +			DBG("unknown ublox model id %d", data->model_id);
> +			goto fail;
> +		}
> +	} else {
> +		data->vendor_family = OFONO_VENDOR_UBLOX;
> +	}
> +
> +	if (g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix,
> +					cfun_enable, modem, NULL))
> +		return;
> +
> +fail:
> +	g_at_chat_unref(data->aux);
> +	data->aux = NULL;
> +	g_at_chat_unref(data->modem);
> +	data->modem = NULL;
> +	ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +static int ublox_enable(struct ofono_modem *modem)
> +{
> +	struct ublox_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
>   	data->aux = open_device(modem, "Aux", "Aux: ");
>   	/* If this is a serial modem then the device may be behind
>   	 * the 'Device' attribute instead...
> @@ -199,7 +243,6 @@ static int ublox_enable(struct ofono_modem *modem)
>   		g_at_chat_set_slave(data->modem, data->aux);
>   		g_at_chat_send(data->modem, "ATE0 +CMEE=1", none_prefix,
>   						NULL, NULL, NULL);
> -
>   		g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL);
>   	}
>   
> @@ -209,10 +252,14 @@ static int ublox_enable(struct ofono_modem *modem)
>   	g_at_chat_send(data->aux, "ATE0 +CMEE=1", none_prefix,
>   					NULL, NULL, NULL);
>   
> -	g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix,
> -					cfun_enable, modem, NULL);
> +	if (g_at_chat_send(data->aux, "AT+CGMM", NULL,
> +				query_model_cb, modem, NULL) > 0)
> +		return -EINPROGRESS;
>   
> -	return -EINPROGRESS;
> +	g_at_chat_unref(data->aux);
> +	data->aux = NULL;
> +
> +	return -EINVAL;
>   }
>   
>   static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
> 

Regards,
-Denis

Re: [PATCH 03/14] ublox: make device selection more flexible

[Denis Kenzior]

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

Hi Jonas,

On 03/12/2019 06:09 AM, Jonas Bonn wrote:
> Many ublox modems can sit on either the USB bus or talk directly to a
> UART.  The udev plugin mostly takes care of figuring out what ports to
> talk to and the protocol is common for all devices after that.
> 
> This patch simplifies the setup a bit:
> i)  There must always be an aux channel for communication with the modem
> ii)  The aux channel may be found behind the string Aux for USB modems
> or Device for serial modems
> iii)  If the Modem string is set, use it; if not set, assume it's not
> available.
> ---
>   plugins/ublox.c | 21 ++++++++++-----------
>   1 file changed, 10 insertions(+), 11 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 04/14] ublox: create model data structures

[Denis Kenzior]

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

Hi Jonas,

On 03/12/2019 06:09 AM, Jonas Bonn wrote:
> This sets up some device model data structures and adds some helpers for
> manipulating them.  These data structures are a first step to trying to
> support the large number of devices that ublox produces with a common
> driver structure.
> ---
>   drivers/ubloxmodem/ubloxmodem.c | 55 +++++++++++++++++++++++++++++++++
>   drivers/ubloxmodem/ubloxmodem.h | 14 +++++++++
>   2 files changed, 69 insertions(+)
> 

I fixed up a bunch of style violations and applied this patch.  Thanks!

Regards,
-Denis

Re: [PATCH 05/14] ublox: query device model

[Jonas Bonn]

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

Hi Denis,

Thanks for the review.  I'm a bit baffled by some of the warnings you 
are seeing... DOS line endings?  Where did those come from... I don't 
see them here.

Nonetheless, I'll send a quick resubmission that passes checkpatch.pl 
and we'll take it from there.

Thanks,
Jonas

On 13/03/2019 19:37, Denis Kenzior wrote:
> Hi Jonas,
> 
> On 03/12/2019 06:09 AM, Jonas Bonn wrote:
>> This patch adds a call to CGMM into the modem_enable path in order to
>> establish the specific device model.  From this device model string, a
>> model-specific capabilities structure can be selected.
>> ---
>>   plugins/ublox.c | 107 ++++++++++++++++++++++++++++++++++--------------
>>   1 file changed, 77 insertions(+), 30 deletions(-)
>>
> 
> Have you tried running checkpatch.pl on these?  There are too many 
> coding style violations for me to fix.
> 
> Here's a small sample:
> 
> ERROR: "foo* bar" should be "foo *bar"
> #138: FILE: plugins/ublox.c:64:
> +    const struct ublox_model* model;
> 
> ERROR: DOS line endings
> #147: FILE: plugins/ublox.c:158:
> +static void query_model_cb(gboolean ok, GAtResult *result, gpointer 
> user_data)^M$
> 
> ERROR: DOS line endings
> #149: FILE: plugins/ublox.c:160:
> +^Istruct ofono_modem* modem = user_data;^M$
> 
> ERROR: "foo* bar" should be "foo *bar"
> #149: FILE: plugins/ublox.c:160:
> +    struct ofono_modem* modem = user_data;
> 
> 
> 
>> diff --git a/plugins/ublox.c b/plugins/ublox.c
>> index 32cf1e61..efb0afbc 100644
>> --- a/plugins/ublox.c
>> +++ b/plugins/ublox.c
>> @@ -42,9 +42,10 @@
>>   #include <ofono/lte.h>
>>   #include <ofono/voicecall.h>
>> -#include <drivers/atmodem/atutil.h>
>>   #include <drivers/atmodem/vendor.h>
>> +#include <drivers/ubloxmodem/ubloxmodem.h>
>> +
>>   static const char *none_prefix[] = { NULL };
>>   enum supported_models {
>> @@ -59,6 +60,8 @@ struct ublox_data {
>>       GAtChat *aux;
>>       int model_id;
>>       enum ofono_vendor vendor_family;
>> +
>> +    const struct ublox_model* model;
>>   };
>>   static void ublox_debug(const char *str, void *user_data)
>> @@ -152,38 +155,79 @@ static void cfun_enable(gboolean ok, GAtResult 
>> *result, gpointer user_data)
>>       ofono_modem_set_powered(modem, TRUE);
>>   }
>> -static int ublox_enable(struct ofono_modem *modem)
>> +static void query_model_cb(gboolean ok, GAtResult *result, gpointer 
>> user_data)
>>   {
>> +    struct ofono_modem* modem = user_data;
>>       struct ublox_data *data = ofono_modem_get_data(modem);
>> -    const char *model_str = NULL;
>> +    struct ofono_error error;
>> +    const char *model;
>> +    const struct ublox_model* m;
>> +    const char* model_str;
>> -    DBG("%p", modem);
>> +    decode_at_error(&error, g_at_result_final_response(result));
>> -    model_str = ofono_modem_get_string(modem, "Model");
>> -    if (model_str == NULL)
>> -        return -EINVAL;
>> +    if (!ok)
>> +        goto fail;
>> -    /*
>> -     * Toby L2 devices are more complex and special than previously
>> -     * supported U-Blox devices. So they need a vendor of their own.
>> -     */
>> -    data->model_id = atoi(model_str);
>> -
>> -    data->vendor_family = OFONO_VENDOR_UBLOX;
>> -
>> -    switch (data->model_id) {
>> -    case SARA_G270:
>> -    case TOBYL2_COMPATIBLE_MODE:
>> -    case TOBYL2_HIGH_THROUGHPUT_MODE:
>> -        break;
>> -    case TOBYL2_MEDIUM_THROUGHPUT_MODE:
>> -        DBG("low/medium throughtput profile unsupported");
>> -        break;
>> -    default:
>> -        DBG("unknown ublox model id %d", data->model_id);
>> -        return -EINVAL;
>> +    if (at_util_parse_attr(result, "", &model) == FALSE) {
>> +        ofono_error("Failed to query modem model");
>> +        goto fail;
>>       }
>> +    m = ublox_model_from_name(model);
>> +    if (!m->name) {
> 
> So you just potentially crashed here
> 
>> +        ofono_error("Unrecognized model: %s", model);
>> +        goto fail;
>> +    }
>> +
>> +    data->model = m;
>> +
>> +    DBG("Model: %s", data->model->name);
>> +
>> +    if (data->model->flags & UBLOX_F_TOBY_L2) {
>> +        model_str = ofono_modem_get_string(modem, "Model");
>> +        if (!model_str)
>> +            goto fail;
>> +
>> +        /*
>> +         * Toby L2 devices are more complex and special than previously
>> +         * supported U-Blox devices. So they need a vendor of their own.
>> +         */
>> +        data->model_id = atoi(model_str);
>> +
> 
> atoi is generally considered harmful.  Use strtoul instead
> 
>> +        switch (data->model_id) {
>> +        case TOBYL2_COMPATIBLE_MODE:
>> +        case TOBYL2_HIGH_THROUGHPUT_MODE:
>> +            break;
>> +        case TOBYL2_MEDIUM_THROUGHPUT_MODE:
>> +            DBG("low/medium throughtput profile unsupported");
>> +            break;
>> +        default:
>> +            DBG("unknown ublox model id %d", data->model_id);
>> +            goto fail;
>> +        }
>> +    } else {
>> +        data->vendor_family = OFONO_VENDOR_UBLOX;
>> +    }
>> +
>> +    if (g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix,
>> +                    cfun_enable, modem, NULL))
>> +        return;
>> +
>> +fail:
>> +    g_at_chat_unref(data->aux);
>> +    data->aux = NULL;
>> +    g_at_chat_unref(data->modem);
>> +    data->modem = NULL;
>> +    ofono_modem_set_powered(modem, FALSE);
>> +}
>> +
>> +static int ublox_enable(struct ofono_modem *modem)
>> +{
>> +    struct ublox_data *data = ofono_modem_get_data(modem);
>> +
>> +    DBG("%p", modem);
>> +
>>       data->aux = open_device(modem, "Aux", "Aux: ");
>>       /* If this is a serial modem then the device may be behind
>>        * the 'Device' attribute instead...
>> @@ -199,7 +243,6 @@ static int ublox_enable(struct ofono_modem *modem)
>>           g_at_chat_set_slave(data->modem, data->aux);
>>           g_at_chat_send(data->modem, "ATE0 +CMEE=1", none_prefix,
>>                           NULL, NULL, NULL);
>> -
>>           g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL);
>>       }
>> @@ -209,10 +252,14 @@ static int ublox_enable(struct ofono_modem *modem)
>>       g_at_chat_send(data->aux, "ATE0 +CMEE=1", none_prefix,
>>                       NULL, NULL, NULL);
>> -    g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix,
>> -                    cfun_enable, modem, NULL);
>> +    if (g_at_chat_send(data->aux, "AT+CGMM", NULL,
>> +                query_model_cb, modem, NULL) > 0)
>> +        return -EINPROGRESS;
>> -    return -EINPROGRESS;
>> +    g_at_chat_unref(data->aux);
>> +    data->aux = NULL;
>> +
>> +    return -EINVAL;
>>   }
>>   static void cfun_disable(gboolean ok, GAtResult *result, gpointer 
>> user_data)
>>
> 
> Regards,
> -Denis