[PATCHv3 0/9] Quectel M95 support

[PATCHv3 0/9] Quectel M95 support

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

Changes since v1/v2:
 * remove c types patch
 * add patch with m95 sms quirks
 * add patch to query device model
 * add patch to create voicecall, phonebook, and sms functionality
 * add patch to enable flow control on modem if configured

Martin Hundebøll (9):
  atmodem: sms: add quectel m95 quirks
  quectel: always print modem pointer in debug messages
  quectel: unwrap call to at_util_open_device()
  quectel: replace glib uses with ell
  quectel: add basic support for serial connected modems
  quectel: support gpio to power on/off the modem
  quectel: configure flow control when enabled
  quectel: query device model to enable vendor quirks
  quectel: initialize call, phonebook and sms when ready

 drivers/atmodem/sms.c |  24 ++-
 plugins/quectel.c     | 491 ++++++++++++++++++++++++++++++++++++++----
 plugins/udevng.c      |  32 ++-
 3 files changed, 494 insertions(+), 53 deletions(-)

-- 
2.22.0

[PATCHv3 1/9] atmodem: sms: add quectel m95 quirks

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

CNMA isn't mentioned in the m95 documentation, but trial'n'error has
revealed some details:
 * the CSMS query returns the list (0,128) instead of a range
 * CNMA is enabled by setting 128 as CSMS service
 * once enabled, SMS deliveries are acked by sending AT+CNMA without a
   value setting

Add m95 quirks to the atmodem driver, so that CNMA is correctly
detected, configured, and used.
---
 drivers/atmodem/sms.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/atmodem/sms.c b/drivers/atmodem/sms.c
index 2f869da6..c3c82afb 100644
--- a/drivers/atmodem/sms.c
+++ b/drivers/atmodem/sms.c
@@ -339,6 +339,9 @@ static inline void at_ack_delivery(struct ofono_sms *sms)
 		case OFONO_VENDOR_GEMALTO:
 			snprintf(buf, sizeof(buf), "AT+CNMA=1");
 			break;
+		case OFONO_VENDOR_QUECTEL_M95:
+			snprintf(buf, sizeof(buf), "AT+CNMA");
+			break;
 		default:
 			snprintf(buf, sizeof(buf), "AT+CNMA=1,%d\r%s",
 					data->cnma_ack_pdu_len,
@@ -1238,7 +1241,7 @@ static void at_csms_status_cb(gboolean ok, GAtResult *result,
 		if (!g_at_result_iter_next_number(&iter, &mo))
 			goto out;
 
-		if (service == 1)
+		if (service == 1 || service == 128)
 			data->cnma_enabled = TRUE;
 
 		if (mt == 1 && mo == 1)
@@ -1269,10 +1272,10 @@ static void at_csms_query_cb(gboolean ok, GAtResult *result,
 {
 	struct ofono_sms *sms = user_data;
 	struct sms_data *data = ofono_sms_get_data(sms);
-	gboolean cnma_supported = FALSE;
 	GAtResultIter iter;
 	int status_min, status_max;
 	char buf[128];
+	int csms = 0;
 
 	if (!ok)
 		return at_sms_not_supported(sms);
@@ -1285,14 +1288,25 @@ static void at_csms_query_cb(gboolean ok, GAtResult *result,
 	if (!g_at_result_iter_open_list(&iter))
 		goto out;
 
-	while (g_at_result_iter_next_range(&iter, &status_min, &status_max))
+	switch (data->vendor) {
+	case OFONO_VENDOR_QUECTEL_M95:
+		g_at_result_iter_next_number(&iter, &status_min);
+		g_at_result_iter_next_number(&iter, &status_max);
 		if (status_min <= 1 && 1 <= status_max)
-			cnma_supported = TRUE;
+			csms = 128;
+		break;
+	default:
+		while (g_at_result_iter_next_range(&iter, &status_min,
+							&status_max))
+			if (status_min <= 1 && 1 <= status_max)
+				csms = 1;
+		break;
+	}
 
 	DBG("CSMS query parsed successfully");
 
 out:
-	snprintf(buf, sizeof(buf), "AT+CSMS=%d", cnma_supported ? 1 : 0);
+	snprintf(buf, sizeof(buf), "AT+CSMS=%d", csms);
 	g_at_chat_send(data->chat, buf, csms_prefix,
 			at_csms_set_cb, sms, NULL);
 }
-- 
2.22.0

[PATCHv3 2/9] quectel: always print modem pointer in debug messages

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

---
 plugins/quectel.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 37969097..e1a5ffa4 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -127,7 +127,7 @@ static void cpin_notify(GAtResult *result, gpointer user_data)
 
 static void cpin_query(gboolean ok, GAtResult *result, gpointer user_data)
 {
-	DBG("ok %d", ok);
+	DBG("%p ok %d", user_data, ok);
 
 	if (ok)
 		cpin_notify(result, user_data);
@@ -138,7 +138,7 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_modem *modem = user_data;
 	struct quectel_data *data = ofono_modem_get_data(modem);
 
-	DBG("ok %d", ok);
+	DBG("%p ok %d", modem, ok);
 
 	if (!ok) {
 		g_at_chat_unref(data->aux);
@@ -162,7 +162,7 @@ static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
 	GAtResultIter iter;
 	int status;
 
-	DBG("ok %d", ok);
+	DBG("%p ok %d", modem, ok);
 
 	if (!ok)
 		return;
@@ -227,7 +227,7 @@ static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_modem *modem = user_data;
 	struct quectel_data *data = ofono_modem_get_data(modem);
 
-	DBG("");
+	DBG("%p", modem);
 
 	g_at_chat_unref(data->aux);
 	data->aux = NULL;
@@ -263,6 +263,8 @@ static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_modem_online_cb_t cb = cbd->cb;
 	struct ofono_error error;
 
+	DBG("%p", user_data);
+
 	decode_at_error(&error, g_at_result_final_response(result));
 	cb(&error, cbd->data);
 }
@@ -319,6 +321,8 @@ static void quectel_post_online(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
 
+	DBG("%p", modem);
+
 	ofono_netreg_create(modem, 0, "atmodem", data->aux);
 }
 
-- 
2.22.0

[PATCHv3 3/9] quectel: unwrap call to at_util_open_device()

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

---
 plugins/quectel.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index e1a5ffa4..faad16cc 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -91,12 +91,6 @@ static void quectel_remove(struct ofono_modem *modem)
 	g_free(data);
 }
 
-static GAtChat *open_device(struct ofono_modem *modem, const char *key,
-				char *debug)
-{
-	return at_util_open_device(modem, key, quectel_debug, debug, NULL);
-}
-
 static void cpin_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -199,11 +193,13 @@ static int quectel_enable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	data->modem = open_device(modem, "Modem", "Modem: ");
+	data->modem = at_util_open_device(modem, "Modem", quectel_debug,
+						"Modem: ", NULL);
 	if (data->modem == NULL)
 		return -EINVAL;
 
-	data->aux = open_device(modem, "Aux", "Aux: ");
+	data->aux = at_util_open_device(modem, "Aux", quectel_debug, "Aux: ",
+					NULL);
 	if (data->aux == NULL) {
 		g_at_chat_unref(data->modem);
 		data->modem = NULL;
-- 
2.22.0

[PATCHv3 4/9] quectel: replace glib uses with ell

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

---

Changes since v1:
 * don't use l_free in plugin callback

 plugins/quectel.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index faad16cc..6e93f45a 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -26,7 +26,7 @@
 #include <errno.h>
 #include <stdlib.h>
 
-#include <glib.h>
+#include <ell/ell.h>
 #include <gatchat.h>
 #include <gattty.h>
 
@@ -67,9 +67,7 @@ static int quectel_probe(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	data = g_try_new0(struct quectel_data, 1);
-	if (data == NULL)
-		return -ENOMEM;
+	data = l_new(struct quectel_data, 1);
 
 	ofono_modem_set_data(modem, data);
 
@@ -88,7 +86,7 @@ static void quectel_remove(struct ofono_modem *modem)
 	ofono_modem_set_data(modem, NULL);
 	g_at_chat_unref(data->aux);
 	g_at_chat_unref(data->modem);
-	g_free(data);
+	l_free(data);
 }
 
 static void cpin_notify(GAtResult *result, gpointer user_data)
-- 
2.22.0

[PATCHv3 5/9] quectel: add basic support for serial connected modems

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

Setup GSM 07.10 multiplexing using the kernel n_gsm line discpline
driver, and use the virtual tty devices as Aux and Modem channels.

The driver supports rts/cts on the underlying serial device. This is
enabled with OFONO_QUECTED_RTSCTS udev environment, e.g.:

KERNEL=="ttymxc0", ENV{OFONO_DRIVER}="quectel", \
        ENV{OFONO_QUECTEL_RTSCTS}="on"
---

Changes since v1:
 * work around kernel dead-lock in n_gsm tty line discipline
 * clearify explanatory comment in cfun_query()
 * add error handling to cfun_query()
 * move hard-coding of gsm tty paths from udevng to quectel

 plugins/quectel.c | 275 ++++++++++++++++++++++++++++++++++++++++------
 plugins/udevng.c  |  24 +++-
 2 files changed, 267 insertions(+), 32 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 6e93f45a..8914cf45 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -25,7 +25,13 @@
 
 #include <errno.h>
 #include <stdlib.h>
+#include <stdbool.h>
+#include <unistd.h>
 
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <linux/tty.h>
+#include <linux/gsmmux.h>
 #include <ell/ell.h>
 #include <gatchat.h>
 #include <gattty.h>
@@ -47,11 +53,28 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *cpin_prefix[] = { "+CPIN:", NULL };
 static const char *none_prefix[] = { NULL };
 
+static const uint8_t gsm0710_terminate[] = {
+	0xf9, /* open flag */
+	0x03, /* channel 0 */
+	0xef, /* UIH frame */
+	0x05, /* 2 data bytes */
+	0xc3, /* terminate 1 */
+	0x01, /* terminate 2 */
+	0xf2, /* crc */
+	0xf9, /* close flag */
+};
+
 struct quectel_data {
 	GAtChat *modem;
 	GAtChat *aux;
 	guint cpin_ready;
-	gboolean have_sim;
+	bool have_sim;
+
+	/* used by quectel uart driver */
+	GAtChat *uart;
+	struct l_timeout *mux_ready_timer;
+	int mux_ready_count;
+	int initial_ldisc;
 };
 
 static void quectel_debug(const char *str, void *user_data)
@@ -86,9 +109,67 @@ static void quectel_remove(struct ofono_modem *modem)
 	ofono_modem_set_data(modem, NULL);
 	g_at_chat_unref(data->aux);
 	g_at_chat_unref(data->modem);
+	g_at_chat_unref(data->uart);
 	l_free(data);
 }
 
+static void close_mux_cb(struct l_timeout *timeout, void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+	GIOChannel *device;
+	uint32_t gpio_value = 0;
+	ssize_t write_count;
+	int fd;
+
+	DBG("%p", modem);
+
+	device = g_at_chat_get_channel(data->uart);
+	fd = g_io_channel_unix_get_fd(device);
+
+	/* restore initial tty line discipline */
+	if (ioctl(fd, TIOCSETD, &data->initial_ldisc) < 0)
+		ofono_warn("Failed to restore line discipline");
+
+	/* terminate gsm 0710 multiplexing on the modem side */
+	write_count = write(fd, gsm0710_terminate, sizeof(gsm0710_terminate));
+	if (write_count != sizeof(gsm0710_terminate))
+		ofono_warn("Failed to terminate gsm multiplexing");
+
+	g_at_chat_unref(data->uart);
+	data->uart = NULL;
+
+	l_timeout_remove(timeout);
+	ofono_modem_set_powered(modem, false);
+}
+
+static void close_serial(struct ofono_modem *modem)
+{
+	struct quectel_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	g_at_chat_unref(data->aux);
+	data->aux = NULL;
+
+	g_at_chat_unref(data->modem);
+	data->modem = NULL;
+
+	/*
+	 * if gsm0710 multiplexing is used, the aux and modem file descriptors
+	 * must be closed before closing the underlying serial device to avoid
+	 * an old kernel dead-lock:
+	 * https://lists.ofono.org/pipermail/ofono/2011-March/009405.html
+	 *
+	 * setup a timer to iterate the mainloop once to let gatchat close the
+	 * virtual file descriptors unreferenced above
+	 */
+	if (data->uart)
+		l_timeout_create_ms(1, close_mux_cb, modem, NULL);
+	else
+		ofono_modem_set_powered(modem, false);
+}
+
 static void cpin_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -106,7 +187,7 @@ static void cpin_notify(GAtResult *result, gpointer user_data)
 	g_at_result_iter_next_unquoted_string(&iter, &sim_inserted);
 
 	if (g_strcmp0(sim_inserted, "NOT INSERTED") != 0)
-		data->have_sim = TRUE;
+		data->have_sim = true;
 
 	ofono_modem_set_powered(modem, TRUE);
 
@@ -133,11 +214,7 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 	DBG("%p ok %d", modem, ok);
 
 	if (!ok) {
-		g_at_chat_unref(data->aux);
-		data->aux = NULL;
-		g_at_chat_unref(data->modem);
-		data->modem = NULL;
-		ofono_modem_set_powered(modem, FALSE);
+		close_serial(modem);
 		return;
 	}
 
@@ -152,19 +229,23 @@ static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_modem *modem = user_data;
 	struct quectel_data *data = ofono_modem_get_data(modem);
 	GAtResultIter iter;
-	int status;
+	int cfun;
 
 	DBG("%p ok %d", modem, ok);
 
-	if (!ok)
+	if (!ok) {
+		close_serial(modem);
 		return;
+	}
 
 	g_at_result_iter_init(&iter, result);
 
-	if (g_at_result_iter_next(&iter, "+CFUN:") == FALSE)
+	if (g_at_result_iter_next(&iter, "+CFUN:") == FALSE) {
+		close_serial(modem);
 		return;
+	}
 
-	g_at_result_iter_next_number(&iter, &status);
+	g_at_result_iter_next_number(&iter, &cfun);
 
 	/*
 	 * The modem firmware powers up in CFUN=1 but will respond to AT+CFUN=4
@@ -172,20 +253,18 @@ static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
 	 * passes.  Empirical evidence suggests that the firmware will report an
 	 * unsolicited +CPIN: notification when it is ready to be useful.
 	 *
-	 * Work around this feature by only transitioning to CFUN=4 after we've
-	 * received an unsolicited +CPIN: notification.
+	 * Work around this feature by only transitioning to CFUN=4 if the
+	 * modem is not in CFUN=1 or untill after we've received an unsolicited
+	 * +CPIN: notification.
 	 */
-
-	if (status != 1) {
+	if (cfun != 1)
 		g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix, cfun_enable,
 				modem, NULL);
-		return;
-	}
-
-	cfun_enable(TRUE, NULL, modem);
+	else
+		cfun_enable(TRUE, NULL, modem);
 }
 
-static int quectel_enable(struct ofono_modem *modem)
+static int open_ttys(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
 
@@ -216,18 +295,155 @@ static int quectel_enable(struct ofono_modem *modem)
 	return -EINPROGRESS;
 }
 
-static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
+static void mux_ready_cb(struct l_timeout *timeout, void *user_data)
 {
 	struct ofono_modem *modem = user_data;
 	struct quectel_data *data = ofono_modem_get_data(modem);
+	struct stat st;
+	int i, ret;
 
 	DBG("%p", modem);
 
-	g_at_chat_unref(data->aux);
-	data->aux = NULL;
+	/* check if the last (and thus all) virtual gsm tty's are created */
+	ret = stat(ofono_modem_get_string(modem, "Modem"), &st);
+	if (ret < 0) {
+		if (data->mux_ready_count++ < 5) {
+			/* not ready yet; try again in 100 ms*/
+			l_timeout_modify_ms(timeout, 100);
+			return;
+		}
+
+		/* not ready after 500 ms; bail out */
+		close_serial(modem);
+		return;
+	}
 
-	if (ok)
-		ofono_modem_set_powered(modem, FALSE);
+	/* virtual gsm tty's are ready */
+	l_timeout_remove(timeout);
+
+	if (open_ttys(modem) != -EINPROGRESS)
+		close_serial(modem);
+
+	g_at_chat_set_slave(data->uart, data->modem);
+}
+
+static void cmux_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+	struct gsm_config gsm_config;
+	GIOChannel *device;
+	int ldisc = N_GSM0710;
+	int fd;
+
+	DBG("%p", modem);
+
+	device = g_at_chat_get_channel(data->uart);
+	fd = g_io_channel_unix_get_fd(device);
+
+	/* get initial line discipline to restore after use */
+	if (ioctl(fd, TIOCGETD, &data->initial_ldisc) < 0) {
+		ofono_error("Failed to get current line discipline: %s", strerror(errno));
+		close_serial(modem);
+		return;
+	}
+
+	/* enable gsm 0710 multiplexing line discipline */
+	if (ioctl(fd, TIOCSETD, &ldisc) < 0) {
+		ofono_error("Failed to set multiplexer line discipline: %s", strerror(errno));
+		close_serial(modem);
+		return;
+	}
+
+	/* get n_gsm configuration */
+	if (ioctl(fd, GSMIOC_GETCONF, &gsm_config) < 0) {
+		ofono_error("Failed to get gsm config: %s", strerror(errno));
+		close_serial(modem);
+		return;
+	}
+
+	gsm_config.initiator = 1;     /* cpu side is initiating multiplexing */
+	gsm_config.encapsulation = 0; /* basic transparency encoding */
+	gsm_config.mru = 127;         /* 127 bytes rx mtu */
+	gsm_config.mtu = 127;         /* 127 bytes tx mtu */
+	gsm_config.t1 = 10;           /* 100 ms ack timer */
+	gsm_config.n2 = 3;            /* 3 retries */
+	gsm_config.t2 = 30;           /* 300 ms response timer */
+	gsm_config.t3 = 10;           /* 100 ms wake up response timer */
+	gsm_config.i = 1;             /* subset */
+
+	/* set the new configuration */
+	if (ioctl(fd, GSMIOC_SETCONF, &gsm_config) < 0) {
+		ofono_error("Failed to set gsm config: %s", strerror(errno));
+		close_serial(modem);
+		return;
+	}
+
+	/*
+	 * the kernel does not yet support mapping the underlying serial device
+	 * to its virtual gsm ttys, so hard-code gsmtty1 gsmtty2 for now
+	 */
+	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty1");
+	ofono_modem_set_string(modem, "Modem", "/dev/gsmtty2");
+
+	/* wait for gsmtty devices to appear */
+	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
+		close_serial(modem);
+		return;
+	}
+}
+
+static int open_serial(struct ofono_modem *modem)
+{
+	struct quectel_data *data = ofono_modem_get_data(modem);
+	const char *rts_cts;
+
+	DBG("%p", modem);
+
+	rts_cts = ofono_modem_get_string(modem, "RtsCts");
+
+	data->uart = at_util_open_device(modem, "Device", quectel_debug,
+						"UART: ",
+						"Baud", "115200",
+						"Parity", "none",
+						"StopBits", "1",
+						"DataBits", "8",
+						"XonXoff", "off",
+						"Local", "on",
+						"Read", "on",
+						"RtsCts", rts_cts,
+						NULL);
+	if (data->uart == NULL)
+		return -EINVAL;
+
+	g_at_chat_send(data->uart, "ATE0", none_prefix, NULL, NULL,
+			NULL);
+
+	/* setup multiplexing */
+	g_at_chat_send(data->uart, "AT+CMUX=0,0,5,127,10,3,30,10,2", NULL,
+			cmux_cb, modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+static int quectel_enable(struct ofono_modem *modem)
+{
+	DBG("%p", modem);
+
+	if (ofono_modem_get_string(modem, "Device"))
+		return open_serial(modem);
+	else
+		return open_ttys(modem);
+}
+
+static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	close_serial(modem);
 }
 
 static int quectel_disable(struct ofono_modem *modem)
@@ -239,14 +455,11 @@ static int quectel_disable(struct ofono_modem *modem)
 	g_at_chat_cancel_all(data->modem);
 	g_at_chat_unregister_all(data->modem);
 
-	g_at_chat_unref(data->modem);
-	data->modem = NULL;
-
 	g_at_chat_cancel_all(data->aux);
 	g_at_chat_unregister_all(data->aux);
 
-	g_at_chat_send(data->aux, "AT+CFUN=0", cfun_prefix,
-					cfun_disable, modem, NULL);
+	g_at_chat_send(data->aux, "AT+CFUN=0", cfun_prefix, cfun_disable, modem,
+			NULL);
 
 	return -EINPROGRESS;
 }
@@ -292,7 +505,7 @@ static void quectel_pre_sim(struct ofono_modem *modem)
 	sim = ofono_sim_create(modem, OFONO_VENDOR_QUECTEL, "atmodem",
 				data->aux);
 
-	if (sim && data->have_sim == TRUE)
+	if (sim && data->have_sim == true)
 		ofono_sim_inserted_notify(sim, TRUE);
 }
 
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 4b420dc0..567196bc 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -837,7 +837,7 @@ static gboolean setup_samsung(struct modem_info *modem)
 	return TRUE;
 }
 
-static gboolean setup_quectel(struct modem_info *modem)
+static gboolean setup_quectel_usb(struct modem_info *modem)
 {
 	const char *aux = NULL, *mdm = NULL;
 	GSList *list;
@@ -877,6 +877,28 @@ static gboolean setup_quectel(struct modem_info *modem)
 	return TRUE;
 }
 
+static gboolean setup_quectel_serial(struct modem_info *modem)
+{
+	struct serial_device_info* info;
+	const char *value;
+
+	info = modem->serial;
+	value = udev_device_get_property_value(info->dev, "OFONO_QUECTEL_RTSCTS");
+
+	ofono_modem_set_string(modem->modem, "RtsCts", value ? value : "off");
+	ofono_modem_set_string(modem->modem, "Device", info->devnode);
+
+	return TRUE;
+}
+
+static gboolean setup_quectel(struct modem_info *modem)
+{
+	if (modem->serial)
+		return setup_quectel_serial(modem);
+	else
+		return setup_quectel_usb(modem);
+}
+
 static gboolean setup_quectelqmi(struct modem_info *modem)
 {
 	const char *qmi = NULL, *net = NULL, *gps = NULL, *aux = NULL;
-- 
2.22.0

[PATCHv3 6/9] quectel: support gpio to power on/off the modem

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

This adds support for configuring a gpio in udev to control the modem
power.

To enable gpio control, specify OFONO_QUECTEL_GPIO_CHIP and
OFONO_QUECTEL_GPIO_OFFSET in the udev environment, for example:

KERNEL=="ttymxc0", ENV{OFONO_DRIVER}="quectel", \
        ENV{OFONO_QUECTEL_GPIO_CHIP}="gpiochip2", \
        ENV{OFONO_QUECTEL_GPIO_OFFSET}="26"
---

Changes since v2:
 * fix memleak in quectel_probe_gpio()
 * expand comment to g_at_chat_set_wakeup_command()

 plugins/quectel.c | 84 +++++++++++++++++++++++++++++++++++++++++++----
 plugins/udevng.c  | 10 +++++-
 2 files changed, 87 insertions(+), 7 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 8914cf45..0533164d 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -75,6 +75,7 @@ struct quectel_data {
 	struct l_timeout *mux_ready_timer;
 	int mux_ready_count;
 	int initial_ldisc;
+	struct l_gpio_writer *gpio;
 };
 
 static void quectel_debug(const char *str, void *user_data)
@@ -84,6 +85,43 @@ static void quectel_debug(const char *str, void *user_data)
 	ofono_info("%s%s", prefix, str);
 }
 
+static int quectel_probe_gpio(struct ofono_modem *modem)
+{
+	struct quectel_data *data = ofono_modem_get_data(modem);
+	struct l_gpio_chip *gpiochip;
+	uint32_t offset;
+	const char *chip_name, *offset_str;
+	uint32_t value = 0;
+
+	DBG("%p", modem);
+
+	chip_name = ofono_modem_get_string(modem, "GpioChip");
+	if (!chip_name)
+		return 0;
+
+	offset_str = ofono_modem_get_string(modem, "GpioOffset");
+	if (!offset_str)
+		return -EINVAL;
+
+	offset = strtoul(offset_str, NULL, 0);
+	if (!offset)
+		return -EINVAL;
+
+	gpiochip = l_gpio_chip_new(chip_name);
+	if (!gpiochip)
+		return -ENODEV;
+
+	data->gpio = l_gpio_writer_new(gpiochip, "ofono", 1, &offset,
+						&value);
+
+	l_gpio_chip_free(gpiochip);
+
+	if (!data->gpio)
+		return -EIO;
+
+	return 0;
+}
+
 static int quectel_probe(struct ofono_modem *modem)
 {
 	struct quectel_data *data;
@@ -94,7 +132,7 @@ static int quectel_probe(struct ofono_modem *modem)
 
 	ofono_modem_set_data(modem, data);
 
-	return 0;
+	return quectel_probe_gpio(modem);
 }
 
 static void quectel_remove(struct ofono_modem *modem)
@@ -107,6 +145,7 @@ static void quectel_remove(struct ofono_modem *modem)
 		g_at_chat_unregister(data->aux, data->cpin_ready);
 
 	ofono_modem_set_data(modem, NULL);
+	l_gpio_writer_free(data->gpio);
 	g_at_chat_unref(data->aux);
 	g_at_chat_unref(data->modem);
 	g_at_chat_unref(data->uart);
@@ -140,6 +179,7 @@ static void close_mux_cb(struct l_timeout *timeout, void *user_data)
 	data->uart = NULL;
 
 	l_timeout_remove(timeout);
+	l_gpio_writer_set(data->gpio, 1, &gpio_value);
 	ofono_modem_set_powered(modem, false);
 }
 
@@ -393,9 +433,22 @@ static void cmux_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	}
 }
 
+static void ate_cb(int ok, GAtResult *result, void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	g_at_chat_set_wakeup_command(data->uart, NULL, 0, 0);
+	g_at_chat_send(data->uart, "AT+CMUX=0,0,5,127,10,3,30,10,2", NULL,
+			cmux_cb, modem, NULL);
+}
+
 static int open_serial(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
+	const uint32_t gpio_value = 1;
 	const char *rts_cts;
 
 	DBG("%p", modem);
@@ -416,12 +469,31 @@ static int open_serial(struct ofono_modem *modem)
 	if (data->uart == NULL)
 		return -EINVAL;
 
-	g_at_chat_send(data->uart, "ATE0", none_prefix, NULL, NULL,
-			NULL);
+	if (data->gpio && !l_gpio_writer_set(data->gpio, 1, &gpio_value)) {
+		close_serial(modem);
+		return -EIO;
+	}
 
-	/* setup multiplexing */
-	g_at_chat_send(data->uart, "AT+CMUX=0,0,5,127,10,3,30,10,2", NULL,
-			cmux_cb, modem, NULL);
+	/*
+	 * there are three different power-up scenarios:
+	 *
+	 *  1) the gpio has just been toggled on, so the modem is not ready
+	 *     until it prints RDY
+	 *
+	 *  2) the modem has been on for a while and ready to respond to
+	 *     commands, so there will be no RDY notification
+	 *
+	 *  3) either of the previous to scenarious is the case, but the modem
+	 *     UART is not configured to a fixed bitrate. In this case it needs
+	 *     a few 'AT' bytes to detect the host UART bitrate, but the RDY is
+	 *     lost.
+	 *
+	 * the wakeup command feature is (mis)used to support all three
+	 * scenarious by sending AT commands until the modem responds with OK,
+	 * at which point the modem is ready.
+	 */
+	g_at_chat_set_wakeup_command(data->uart, "AT\r", 500, 10000);
+	g_at_chat_send(data->uart, "ATE0", none_prefix, ate_cb, modem, NULL);
 
 	return -EINPROGRESS;
 }
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 567196bc..d754d126 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -883,8 +883,16 @@ static gboolean setup_quectel_serial(struct modem_info *modem)
 	const char *value;
 
 	info = modem->serial;
-	value = udev_device_get_property_value(info->dev, "OFONO_QUECTEL_RTSCTS");
 
+	value = udev_device_get_property_value(info->dev, "OFONO_QUECTEL_GPIO_CHIP");
+	if (value)
+		ofono_modem_set_string(modem->modem, "GpioChip", value);
+
+	value = udev_device_get_property_value(info->dev, "OFONO_QUECTEL_GPIO_OFFSET");
+	if (value)
+		ofono_modem_set_string(modem->modem, "GpioOffset", value);
+
+	value = udev_device_get_property_value(info->dev, "OFONO_QUECTEL_RTSCTS");
 	ofono_modem_set_string(modem->modem, "RtsCts", value ? value : "off");
 	ofono_modem_set_string(modem->modem, "Device", info->devnode);
 
-- 
2.22.0

[PATCHv3 7/9] quectel: configure flow control when enabled

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

---
 plugins/quectel.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 3f9c93f8..bfae31c0 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -495,7 +495,13 @@ static int open_serial(struct ofono_modem *modem)
 	 * at which point the modem is ready.
 	 */
 	g_at_chat_set_wakeup_command(data->uart, "AT\r", 500, 10000);
-	g_at_chat_send(data->uart, "ATE0", none_prefix, ate_cb, modem, NULL);
+
+	if (strcmp(rts_cts, "on") == 0)
+		g_at_chat_send(data->uart, "AT+IFC=2,2; E0", none_prefix,
+				ate_cb, modem, NULL);
+	else
+		g_at_chat_send(data->uart, "ATE0", none_prefix, ate_cb, modem,
+				NULL);
 
 	return -EINPROGRESS;
 }
-- 
2.22.0

[PATCHv3 8/9] quectel: query device model to enable vendor quirks

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

---
 plugins/quectel.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index bfae31c0..8bf9a963 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -51,6 +51,7 @@
 
 static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *cpin_prefix[] = { "+CPIN:", NULL };
+static const char *cgmm_prefix[] = { "UC15", "Quectel_M95", NULL };
 static const char *none_prefix[] = { NULL };
 
 static const uint8_t gsm0710_terminate[] = {
@@ -69,6 +70,7 @@ struct quectel_data {
 	GAtChat *aux;
 	guint cpin_ready;
 	bool have_sim;
+	enum ofono_vendor vendor;
 
 	/* used by quectel uart driver */
 	GAtChat *uart;
@@ -306,6 +308,36 @@ static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
 		cfun_enable(TRUE, NULL, modem);
 }
 
+static void cgmm_cb(int ok, GAtResult *result, void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	const char *model;
+
+	DBG("%p ok %d", modem, ok);
+
+	if (!at_util_parse_attr(result, "", &model)) {
+		ofono_error("Failed to query modem model");
+		close_serial(modem);
+		return;
+	}
+
+	if (strcmp(model, "UC15") == 0) {
+		DBG("%p model UC15", modem);
+		data->vendor = OFONO_VENDOR_QUECTEL;
+	} else if (strcmp(model, "Quectel_M95") == 0) {
+		DBG("%p model M95", modem);
+		data->vendor = OFONO_VENDOR_QUECTEL_M95;
+	} else {
+		ofono_warn("%p unknown model: '%s'", modem, model);
+		data->vendor = OFONO_VENDOR_QUECTEL;
+	}
+
+	g_at_chat_send(data->aux, "AT+CFUN?", cfun_prefix, cfun_query, modem,
+			NULL);
+}
+
 static int open_ttys(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
@@ -331,7 +363,7 @@ static int open_ttys(struct ofono_modem *modem)
 			NULL, NULL);
 	g_at_chat_send(data->aux, "ATE0; &C0; +CMEE=1", none_prefix, NULL, NULL,
 			NULL);
-	g_at_chat_send(data->aux, "AT+CFUN?", cfun_prefix, cfun_query, modem,
+	g_at_chat_send(data->aux, "AT+CGMM", cgmm_prefix, cgmm_cb, modem,
 			NULL);
 
 	return -EINPROGRESS;
@@ -582,8 +614,7 @@ static void quectel_pre_sim(struct ofono_modem *modem)
 	DBG("%p", modem);
 
 	ofono_devinfo_create(modem, 0, "atmodem", data->aux);
-	sim = ofono_sim_create(modem, OFONO_VENDOR_QUECTEL, "atmodem",
-				data->aux);
+	sim = ofono_sim_create(modem, data->vendor, "atmodem", data->aux);
 
 	if (sim && data->have_sim == true)
 		ofono_sim_inserted_notify(sim, TRUE);
-- 
2.22.0

[PATCHv3 9/9] quectel: initialize call, phonebook and sms when ready

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

The Quectel M95 modem issues a "Call ready" notification when call and
phonebook are ready, so set up a listener for that.

The only way to know when sms is ready is to issue QINITSTAT queries.
Since sms is always ready after call and phonebook, the queries are
initiated after creating call/phonebook.
---
 plugins/quectel.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 8bf9a963..e96a66d7 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -42,6 +42,9 @@
 #include <ofono/devinfo.h>
 #include <ofono/netreg.h>
 #include <ofono/sim.h>
+#include <ofono/sms.h>
+#include <ofono/phonebook.h>
+#include <ofono/voicecall.h>
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
 #include <ofono/log.h>
@@ -51,6 +54,7 @@
 
 static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *cpin_prefix[] = { "+CPIN:", NULL };
+static const char *qinistat_prefix[] = { "+QINISTAT:", NULL };
 static const char *cgmm_prefix[] = { "UC15", "Quectel_M95", NULL };
 static const char *none_prefix[] = { NULL };
 
@@ -69,8 +73,10 @@ struct quectel_data {
 	GAtChat *modem;
 	GAtChat *aux;
 	guint cpin_ready;
+	guint call_ready;
 	bool have_sim;
 	enum ofono_vendor vendor;
+	struct l_timeout *sms_ready_timer;
 
 	/* used by quectel uart driver */
 	GAtChat *uart;
@@ -338,6 +344,67 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 			NULL);
 }
 
+static void qinistat_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	int status;
+
+	DBG("%p", modem);
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+QINISTAT:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &status))
+		return;
+
+	DBG("qinistat: %d", status);
+
+	if (status != 3) {
+		l_timeout_modify_ms(data->sms_ready_timer, 500);
+		return;
+	}
+
+	ofono_sms_create(modem, data->vendor, "atmodem", data->aux);
+	l_timeout_remove(data->sms_ready_timer);
+	data->sms_ready_timer = NULL;
+}
+
+static void sms_ready_cb(struct l_timeout *timeout, void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	g_at_chat_send(data->aux, "AT+QINISTAT", qinistat_prefix, qinistat_cb,
+			modem, NULL);
+}
+
+static void call_ready_notify(GAtResult *result, void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct quectel_data *data = ofono_modem_get_data(modem);
+	const char *sim_inserted;
+	GAtResultIter iter;
+
+	DBG("%p", modem);
+
+	g_at_chat_unregister(data->aux, data->call_ready);
+	data->sms_ready_timer = l_timeout_create_ms(500, sms_ready_cb, modem,
+							NULL);
+	if (!data->sms_ready_timer) {
+		close_serial(modem);
+		return;
+	}
+
+	ofono_phonebook_create(modem, 0, "atmodem", data->aux);
+	ofono_voicecall_create(modem, 0, "atmodem", data->aux);
+}
+
 static int open_ttys(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
@@ -357,6 +424,14 @@ static int open_ttys(struct ofono_modem *modem)
 		return -EIO;
 	}
 
+	data->call_ready = g_at_chat_register(data->aux, "Call Ready",
+						call_ready_notify, false,
+						modem, NULL);
+	if (!data->call_ready) {
+		close_serial(modem);
+		return -ENOTTY;
+	}
+
 	g_at_chat_set_slave(data->modem, data->aux);
 
 	g_at_chat_send(data->modem, "ATE0; &C0; +CMEE=1", none_prefix, NULL,
-- 
2.22.0

Re: [PATCHv3 0/9] Quectel M95 support

[Martin =?unknown-8bit?q?Hundeb=C3=B8ll?=]

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

Hi,

The modem supports an unsolicited notification for voltage level 
warnings and errors. I.e it can send

   UNDER_VOLTAGE POWER DOWN
   UNDER_VOLTAGE WARNING
   OVER_VOLTAGE WARNING
   OVER_VOLTAGE POWER DOWN

The power down messages should probably just close the serial ports and 
call ofono_modem_set_powered(modem, false), but what about the warnings?

Should I just add a hardware monitor dbus interface like the 
gemalto/cinterion drivers do, with a signal for the warnings?

Thanks,
Martin

On 10/07/2019 23.51, Martin Hundebøll wrote:
> Changes since v1/v2:
>   * remove c types patch
>   * add patch with m95 sms quirks
>   * add patch to query device model
>   * add patch to create voicecall, phonebook, and sms functionality
>   * add patch to enable flow control on modem if configured
> 
> Martin Hundebøll (9):
>    atmodem: sms: add quectel m95 quirks
>    quectel: always print modem pointer in debug messages
>    quectel: unwrap call to at_util_open_device()
>    quectel: replace glib uses with ell
>    quectel: add basic support for serial connected modems
>    quectel: support gpio to power on/off the modem
>    quectel: configure flow control when enabled
>    quectel: query device model to enable vendor quirks
>    quectel: initialize call, phonebook and sms when ready
> 
>   drivers/atmodem/sms.c |  24 ++-
>   plugins/quectel.c     | 491 ++++++++++++++++++++++++++++++++++++++----
>   plugins/udevng.c      |  32 ++-
>   3 files changed, 494 insertions(+), 53 deletions(-)
> 

Re: [PATCHv3 1/9] atmodem: sms: add quectel m95 quirks

[Denis Kenzior]

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

Hi Martin,

On 7/10/19 4:51 PM, Martin Hundebøll wrote:
> CNMA isn't mentioned in the m95 documentation, but trial'n'error has
> revealed some details:
>   * the CSMS query returns the list (0,128) instead of a range
>   * CNMA is enabled by setting 128 as CSMS service
>   * once enabled, SMS deliveries are acked by sending AT+CNMA without a
>     value setting
> 
> Add m95 quirks to the atmodem driver, so that CNMA is correctly
> detected, configured, and used.
> ---
>   drivers/atmodem/sms.c | 24 +++++++++++++++++++-----
>   1 file changed, 19 insertions(+), 5 deletions(-)
> 

Patches 1-4 applied, thanks.

Regards,
-Denis

Re: [PATCHv3 0/9] Quectel M95 support

[Denis Kenzior]

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

Hi Martin,

On 7/10/19 5:21 PM, Martin Hundebøll wrote:
> Hi,
> 
> The modem supports an unsolicited notification for voltage level 
> warnings and errors. I.e it can send
> 
>    UNDER_VOLTAGE POWER DOWN
>    UNDER_VOLTAGE WARNING
>    OVER_VOLTAGE WARNING
>    OVER_VOLTAGE POWER DOWN
> 
> The power down messages should probably just close the serial ports and 
> call ofono_modem_set_powered(modem, false), but what about the warnings?

What can be usefully done with these?  I mean, if you just send them to 
syslog, or nowhere at all, will anyone notice?

> 
> Should I just add a hardware monitor dbus interface like the 
> gemalto/cinterion drivers do, with a signal for the warnings?
> 

If you think someone will find this useful, then sure.

Regards,
-Denis

Re: [PATCHv3 5/9] quectel: add basic support for serial connected modems

[Denis Kenzior]

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

Hi Martin,

On 7/10/19 4:51 PM, Martin Hundebøll wrote:
> Setup GSM 07.10 multiplexing using the kernel n_gsm line discpline
> driver, and use the virtual tty devices as Aux and Modem channels.
> 
> The driver supports rts/cts on the underlying serial device. This is
> enabled with OFONO_QUECTED_RTSCTS udev environment, e.g.:
> 
> KERNEL=="ttymxc0", ENV{OFONO_DRIVER}="quectel", \
>          ENV{OFONO_QUECTEL_RTSCTS}="on"
> ---
> 
> Changes since v1:
>   * work around kernel dead-lock in n_gsm tty line discipline
>   * clearify explanatory comment in cfun_query()
>   * add error handling to cfun_query()
>   * move hard-coding of gsm tty paths from udevng to quectel
> 
>   plugins/quectel.c | 275 ++++++++++++++++++++++++++++++++++++++++------
>   plugins/udevng.c  |  24 +++-
>   2 files changed, 267 insertions(+), 32 deletions(-)
> 
> diff --git a/plugins/quectel.c b/plugins/quectel.c
> index 6e93f45a..8914cf45 100644
> --- a/plugins/quectel.c
> +++ b/plugins/quectel.c
> @@ -25,7 +25,13 @@
>   
>   #include <errno.h>
>   #include <stdlib.h>
> +#include <stdbool.h>
> +#include <unistd.h>
>   
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <linux/tty.h>
> +#include <linux/gsmmux.h>
>   #include <ell/ell.h>
>   #include <gatchat.h>
>   #include <gattty.h>
> @@ -47,11 +53,28 @@ static const char *cfun_prefix[] = { "+CFUN:", NULL };
>   static const char *cpin_prefix[] = { "+CPIN:", NULL };
>   static const char *none_prefix[] = { NULL };
>   
> +static const uint8_t gsm0710_terminate[] = {
> +	0xf9, /* open flag */
> +	0x03, /* channel 0 */
> +	0xef, /* UIH frame */
> +	0x05, /* 2 data bytes */
> +	0xc3, /* terminate 1 */
> +	0x01, /* terminate 2 */
> +	0xf2, /* crc */
> +	0xf9, /* close flag */
> +};
> +

I still say all these linux gsm mux details belong in a utility file...

>   struct quectel_data {
>   	GAtChat *modem;
>   	GAtChat *aux;
>   	guint cpin_ready;
> -	gboolean have_sim;
> +	bool have_sim;
> +
> +	/* used by quectel uart driver */
> +	GAtChat *uart;
> +	struct l_timeout *mux_ready_timer;

Removed this unused variable

> +	int mux_ready_count;
> +	int initial_ldisc;
>   };
>   
>   static void quectel_debug(const char *str, void *user_data)
> @@ -86,9 +109,67 @@ static void quectel_remove(struct ofono_modem *modem)
>   	ofono_modem_set_data(modem, NULL);
>   	g_at_chat_unref(data->aux);
>   	g_at_chat_unref(data->modem);
> +	g_at_chat_unref(data->uart);
>   	l_free(data);
>   }
>   
> +static void close_mux_cb(struct l_timeout *timeout, void *user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +	GIOChannel *device;
> +	uint32_t gpio_value = 0;
> +	ssize_t write_count;
> +	int fd;
> +
> +	DBG("%p", modem);
> +
> +	device = g_at_chat_get_channel(data->uart);
> +	fd = g_io_channel_unix_get_fd(device);
> +
> +	/* restore initial tty line discipline */
> +	if (ioctl(fd, TIOCSETD, &data->initial_ldisc) < 0)
> +		ofono_warn("Failed to restore line discipline");
> +
> +	/* terminate gsm 0710 multiplexing on the modem side */
> +	write_count = write(fd, gsm0710_terminate, sizeof(gsm0710_terminate));
> +	if (write_count != sizeof(gsm0710_terminate))
> +		ofono_warn("Failed to terminate gsm multiplexing");
> +
> +	g_at_chat_unref(data->uart);
> +	data->uart = NULL;
> +
> +	l_timeout_remove(timeout);
> +	ofono_modem_set_powered(modem, false);

Changed 'false' to 'FALSE' here since it is an ofono_bool_t

> +}
> +
> +static void close_serial(struct ofono_modem *modem)
> +{
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
> +	g_at_chat_unref(data->aux);
> +	data->aux = NULL;
> +
> +	g_at_chat_unref(data->modem);
> +	data->modem = NULL;
> +
> +	/*
> +	 * if gsm0710 multiplexing is used, the aux and modem file descriptors
> +	 * must be closed before closing the underlying serial device to avoid
> +	 * an old kernel dead-lock:
> +	 * https://lists.ofono.org/pipermail/ofono/2011-March/009405.html
> +	 *
> +	 * setup a timer to iterate the mainloop once to let gatchat close the
> +	 * virtual file descriptors unreferenced above
> +	 */
> +	if (data->uart)
> +		l_timeout_create_ms(1, close_mux_cb, modem, NULL);

I wonder if you should be paranoid and track this timer in case 
close_mux_cb is never called.  E.g. if the device is forcefully removed 
(perhaps a usb<->serial converter is used)?

> +	else
> +		ofono_modem_set_powered(modem, false);
> +}
> +
>   static void cpin_notify(GAtResult *result, gpointer user_data)
>   {
>   	struct ofono_modem *modem = user_data;
> @@ -106,7 +187,7 @@ static void cpin_notify(GAtResult *result, gpointer user_data)
>   	g_at_result_iter_next_unquoted_string(&iter, &sim_inserted);
>   
>   	if (g_strcmp0(sim_inserted, "NOT INSERTED") != 0)
> -		data->have_sim = TRUE;
> +		data->have_sim = true;
>   
>   	ofono_modem_set_powered(modem, TRUE);
>   
> @@ -133,11 +214,7 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>   	DBG("%p ok %d", modem, ok);
>   
>   	if (!ok) {
> -		g_at_chat_unref(data->aux);
> -		data->aux = NULL;
> -		g_at_chat_unref(data->modem);
> -		data->modem = NULL;
> -		ofono_modem_set_powered(modem, FALSE);
> +		close_serial(modem);
>   		return;
>   	}
>   
> @@ -152,19 +229,23 @@ static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
>   	struct ofono_modem *modem = user_data;
>   	struct quectel_data *data = ofono_modem_get_data(modem);
>   	GAtResultIter iter;
> -	int status;
> +	int cfun;
>   
>   	DBG("%p ok %d", modem, ok);
>   
> -	if (!ok)
> +	if (!ok) {
> +		close_serial(modem);
>   		return;
> +	}
>   
>   	g_at_result_iter_init(&iter, result);
>   
> -	if (g_at_result_iter_next(&iter, "+CFUN:") == FALSE)
> +	if (g_at_result_iter_next(&iter, "+CFUN:") == FALSE) {
> +		close_serial(modem);
>   		return;
> +	}
>   
> -	g_at_result_iter_next_number(&iter, &status);
> +	g_at_result_iter_next_number(&iter, &cfun);
>   
>   	/*
>   	 * The modem firmware powers up in CFUN=1 but will respond to AT+CFUN=4
> @@ -172,20 +253,18 @@ static void cfun_query(gboolean ok, GAtResult *result, gpointer user_data)
>   	 * passes.  Empirical evidence suggests that the firmware will report an
>   	 * unsolicited +CPIN: notification when it is ready to be useful.
>   	 *
> -	 * Work around this feature by only transitioning to CFUN=4 after we've
> -	 * received an unsolicited +CPIN: notification.
> +	 * Work around this feature by only transitioning to CFUN=4 if the
> +	 * modem is not in CFUN=1 or untill after we've received an unsolicited

Fixed typo: 'untill' -> 'until'

> +	 * +CPIN: notification.
>   	 */
> -
> -	if (status != 1) {
> +	if (cfun != 1)
>   		g_at_chat_send(data->aux, "AT+CFUN=4", none_prefix, cfun_enable,
>   				modem, NULL);
> -		return;
> -	}
> -
> -	cfun_enable(TRUE, NULL, modem);
> +	else
> +		cfun_enable(TRUE, NULL, modem);
>   }
>   
> -static int quectel_enable(struct ofono_modem *modem)
> +static int open_ttys(struct ofono_modem *modem)
>   {
>   	struct quectel_data *data = ofono_modem_get_data(modem);
>   
> @@ -216,18 +295,155 @@ static int quectel_enable(struct ofono_modem *modem)
>   	return -EINPROGRESS;
>   }
>   
> -static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
> +static void mux_ready_cb(struct l_timeout *timeout, void *user_data)
>   {
>   	struct ofono_modem *modem = user_data;
>   	struct quectel_data *data = ofono_modem_get_data(modem);
> +	struct stat st;
> +	int i, ret;
>   
>   	DBG("%p", modem);
>   
> -	g_at_chat_unref(data->aux);
> -	data->aux = NULL;
> +	/* check if the last (and thus all) virtual gsm tty's are created */
> +	ret = stat(ofono_modem_get_string(modem, "Modem"), &st);
> +	if (ret < 0) {
> +		if (data->mux_ready_count++ < 5) {
> +			/* not ready yet; try again in 100 ms*/
> +			l_timeout_modify_ms(timeout, 100);
> +			return;
> +		}
> +
> +		/* not ready after 500 ms; bail out */
> +		close_serial(modem);
> +		return;
> +	}
>   
> -	if (ok)
> -		ofono_modem_set_powered(modem, FALSE);
> +	/* virtual gsm tty's are ready */
> +	l_timeout_remove(timeout);
> +
> +	if (open_ttys(modem) != -EINPROGRESS)
> +		close_serial(modem);
> +
> +	g_at_chat_set_slave(data->uart, data->modem);
> +}
> +
> +static void cmux_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +	struct gsm_config gsm_config;
> +	GIOChannel *device;
> +	int ldisc = N_GSM0710;
> +	int fd;
> +
> +	DBG("%p", modem);
> +
> +	device = g_at_chat_get_channel(data->uart);
> +	fd = g_io_channel_unix_get_fd(device);
> +
> +	/* get initial line discipline to restore after use */
> +	if (ioctl(fd, TIOCGETD, &data->initial_ldisc) < 0) {
> +		ofono_error("Failed to get current line discipline: %s", strerror(errno));

Fixed over 80 chars / line..

> +		close_serial(modem);
> +		return;
> +	}
> +
> +	/* enable gsm 0710 multiplexing line discipline */
> +	if (ioctl(fd, TIOCSETD, &ldisc) < 0) {
> +		ofono_error("Failed to set multiplexer line discipline: %s", strerror(errno));

And here

> +		close_serial(modem);
> +		return;
> +	}
> +
> +	/* get n_gsm configuration */
> +	if (ioctl(fd, GSMIOC_GETCONF, &gsm_config) < 0) {
> +		ofono_error("Failed to get gsm config: %s", strerror(errno));
> +		close_serial(modem);
> +		return;
> +	}
> +
> +	gsm_config.initiator = 1;     /* cpu side is initiating multiplexing */
> +	gsm_config.encapsulation = 0; /* basic transparency encoding */
> +	gsm_config.mru = 127;         /* 127 bytes rx mtu */
> +	gsm_config.mtu = 127;         /* 127 bytes tx mtu */
> +	gsm_config.t1 = 10;           /* 100 ms ack timer */
> +	gsm_config.n2 = 3;            /* 3 retries */
> +	gsm_config.t2 = 30;           /* 300 ms response timer */
> +	gsm_config.t3 = 10;           /* 100 ms wake up response timer */
> +	gsm_config.i = 1;             /* subset */
> +
> +	/* set the new configuration */
> +	if (ioctl(fd, GSMIOC_SETCONF, &gsm_config) < 0) {
> +		ofono_error("Failed to set gsm config: %s", strerror(errno));
> +		close_serial(modem);
> +		return;
> +	}
> +
> +	/*
> +	 * the kernel does not yet support mapping the underlying serial device
> +	 * to its virtual gsm ttys, so hard-code gsmtty1 gsmtty2 for now
> +	 */
> +	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty1");
> +	ofono_modem_set_string(modem, "Modem", "/dev/gsmtty2");
> +
> +	/* wait for gsmtty devices to appear */
> +	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {

Same comment here.  It seems the unused l_timeout variable was intended 
to be used here...

> +		close_serial(modem);
> +		return;
> +	}
> +}
> +
> +static int open_serial(struct ofono_modem *modem)
> +{
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +	const char *rts_cts;
> +
> +	DBG("%p", modem);
> +
> +	rts_cts = ofono_modem_get_string(modem, "RtsCts");
> +
> +	data->uart = at_util_open_device(modem, "Device", quectel_debug,
> +						"UART: ",
> +						"Baud", "115200",
> +						"Parity", "none",
> +						"StopBits", "1",
> +						"DataBits", "8",
> +						"XonXoff", "off",
> +						"Local", "on",
> +						"Read", "on",
> +						"RtsCts", rts_cts,
> +						NULL);
> +	if (data->uart == NULL)
> +		return -EINVAL;
> +
> +	g_at_chat_send(data->uart, "ATE0", none_prefix, NULL, NULL,
> +			NULL);
> +
> +	/* setup multiplexing */
> +	g_at_chat_send(data->uart, "AT+CMUX=0,0,5,127,10,3,30,10,2", NULL,
> +			cmux_cb, modem, NULL);
> +
> +	return -EINPROGRESS;
> +}
> +
> +static int quectel_enable(struct ofono_modem *modem)
> +{
> +	DBG("%p", modem);
> +
> +	if (ofono_modem_get_string(modem, "Device"))
> +		return open_serial(modem);
> +	else
> +		return open_ttys(modem);
> +}
> +
> +static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
> +	close_serial(modem);
>   }
>   
>   static int quectel_disable(struct ofono_modem *modem)
> @@ -239,14 +455,11 @@ static int quectel_disable(struct ofono_modem *modem)
>   	g_at_chat_cancel_all(data->modem);
>   	g_at_chat_unregister_all(data->modem);
>   
> -	g_at_chat_unref(data->modem);
> -	data->modem = NULL;
> -
>   	g_at_chat_cancel_all(data->aux);
>   	g_at_chat_unregister_all(data->aux);
>   
> -	g_at_chat_send(data->aux, "AT+CFUN=0", cfun_prefix,
> -					cfun_disable, modem, NULL);
> +	g_at_chat_send(data->aux, "AT+CFUN=0", cfun_prefix, cfun_disable, modem,
> +			NULL);
>   
>   	return -EINPROGRESS;
>   }
> @@ -292,7 +505,7 @@ static void quectel_pre_sim(struct ofono_modem *modem)
>   	sim = ofono_sim_create(modem, OFONO_VENDOR_QUECTEL, "atmodem",
>   				data->aux);
>   
> -	if (sim && data->have_sim == TRUE)
> +	if (sim && data->have_sim == true)
>   		ofono_sim_inserted_notify(sim, TRUE);
>   }
>   
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 4b420dc0..567196bc 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -837,7 +837,7 @@ static gboolean setup_samsung(struct modem_info *modem)
>   	return TRUE;
>   }
>   
> -static gboolean setup_quectel(struct modem_info *modem)
> +static gboolean setup_quectel_usb(struct modem_info *modem)
>   {
>   	const char *aux = NULL, *mdm = NULL;
>   	GSList *list;
> @@ -877,6 +877,28 @@ static gboolean setup_quectel(struct modem_info *modem)
>   	return TRUE;
>   }
>   
> +static gboolean setup_quectel_serial(struct modem_info *modem)
> +{
> +	struct serial_device_info* info;
> +	const char *value;
> +
> +	info = modem->serial;

Changed this to initialize at declaration instead

> +	value = udev_device_get_property_value(info->dev, "OFONO_QUECTEL_RTSCTS");
> +

Fixed over 80 chars / line.

> +	ofono_modem_set_string(modem->modem, "RtsCts", value ? value : "off");
> +	ofono_modem_set_string(modem->modem, "Device", info->devnode);
> +
> +	return TRUE;
> +}
> +
> +static gboolean setup_quectel(struct modem_info *modem)
> +{
> +	if (modem->serial)
> +		return setup_quectel_serial(modem);
> +	else
> +		return setup_quectel_usb(modem);
> +}
> +
>   static gboolean setup_quectelqmi(struct modem_info *modem)
>   {
>   	const char *qmi = NULL, *net = NULL, *gps = NULL, *aux = NULL;
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCHv3 6/9] quectel: support gpio to power on/off the modem

[Denis Kenzior]

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

Hi Martin,

On 7/10/19 4:51 PM, Martin Hundebøll wrote:
> This adds support for configuring a gpio in udev to control the modem
> power.
> 
> To enable gpio control, specify OFONO_QUECTEL_GPIO_CHIP and
> OFONO_QUECTEL_GPIO_OFFSET in the udev environment, for example:
> 
> KERNEL=="ttymxc0", ENV{OFONO_DRIVER}="quectel", \
>          ENV{OFONO_QUECTEL_GPIO_CHIP}="gpiochip2", \
>          ENV{OFONO_QUECTEL_GPIO_OFFSET}="26"
> ---
> 
> Changes since v2:
>   * fix memleak in quectel_probe_gpio()
>   * expand comment to g_at_chat_set_wakeup_command()
> 
>   plugins/quectel.c | 84 +++++++++++++++++++++++++++++++++++++++++++----
>   plugins/udevng.c  | 10 +++++-
>   2 files changed, 87 insertions(+), 7 deletions(-)

The built-in ell sources were missing gpio.[ch], so I added a commit for 
that.  Also fixed a few > 80 chars / line issues.

Applied, thanks.

Regards,
-Denis

Re: [PATCHv3 7/9] quectel: configure flow control when enabled

[Denis Kenzior]

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

Hi Martin,

On 7/10/19 4:51 PM, Martin Hundebøll wrote:
> ---
>   plugins/quectel.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 

Patch 7 & 8 applied, thanks.

Regards,
-Denis

Re: [PATCHv3 9/9] quectel: initialize call, phonebook and sms when ready

[Denis Kenzior]

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

Hi Martin,

On 7/10/19 4:51 PM, Martin Hundebøll wrote:
> The Quectel M95 modem issues a "Call ready" notification when call and
> phonebook are ready, so set up a listener for that.
> 
> The only way to know when sms is ready is to issue QINITSTAT queries.
> Since sms is always ready after call and phonebook, the queries are
> initiated after creating call/phonebook.
> ---
>   plugins/quectel.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 75 insertions(+)
> 
> diff --git a/plugins/quectel.c b/plugins/quectel.c
> index 8bf9a963..e96a66d7 100644
> --- a/plugins/quectel.c
> +++ b/plugins/quectel.c
> @@ -42,6 +42,9 @@
>   #include <ofono/devinfo.h>
>   #include <ofono/netreg.h>
>   #include <ofono/sim.h>
> +#include <ofono/sms.h>
> +#include <ofono/phonebook.h>
> +#include <ofono/voicecall.h>
>   #include <ofono/gprs.h>
>   #include <ofono/gprs-context.h>
>   #include <ofono/log.h>
> @@ -51,6 +54,7 @@
>   
>   static const char *cfun_prefix[] = { "+CFUN:", NULL };
>   static const char *cpin_prefix[] = { "+CPIN:", NULL };
> +static const char *qinistat_prefix[] = { "+QINISTAT:", NULL };
>   static const char *cgmm_prefix[] = { "UC15", "Quectel_M95", NULL };
>   static const char *none_prefix[] = { NULL };
>   
> @@ -69,8 +73,10 @@ struct quectel_data {
>   	GAtChat *modem;
>   	GAtChat *aux;
>   	guint cpin_ready;
> +	guint call_ready;
>   	bool have_sim;
>   	enum ofono_vendor vendor;
> +	struct l_timeout *sms_ready_timer;
>   
>   	/* used by quectel uart driver */
>   	GAtChat *uart;
> @@ -338,6 +344,67 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
>   			NULL);
>   }
>   
> +static void qinistat_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	int status;
> +
> +	DBG("%p", modem);
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "+QINISTAT:"))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter, &status))
> +		return;
> +
> +	DBG("qinistat: %d", status);
> +
> +	if (status != 3) {
> +		l_timeout_modify_ms(data->sms_ready_timer, 500);
> +		return;
> +	}
> +
> +	ofono_sms_create(modem, data->vendor, "atmodem", data->aux);
> +	l_timeout_remove(data->sms_ready_timer);
> +	data->sms_ready_timer = NULL;
> +}
> +
> +static void sms_ready_cb(struct l_timeout *timeout, void *user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +
> +	DBG("%p", modem);
> +
> +	g_at_chat_send(data->aux, "AT+QINISTAT", qinistat_prefix, qinistat_cb,
> +			modem, NULL);
> +}
> +
> +static void call_ready_notify(GAtResult *result, void *user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct quectel_data *data = ofono_modem_get_data(modem);
> +	const char *sim_inserted;
> +	GAtResultIter iter;

Squashed these two unused variables

> +
> +	DBG("%p", modem);
> +
> +	g_at_chat_unregister(data->aux, data->call_ready);

Being paranoid, I added data->call_ready = 0 here.

> +	data->sms_ready_timer = l_timeout_create_ms(500, sms_ready_cb, modem,
> +							NULL);
> +	if (!data->sms_ready_timer) {
> +		close_serial(modem);
> +		return;
> +	}
> +
> +	ofono_phonebook_create(modem, 0, "atmodem", data->aux);
> +	ofono_voicecall_create(modem, 0, "atmodem", data->aux);
> +}
> +
>   static int open_ttys(struct ofono_modem *modem)
>   {
>   	struct quectel_data *data = ofono_modem_get_data(modem);
> @@ -357,6 +424,14 @@ static int open_ttys(struct ofono_modem *modem)
>   		return -EIO;
>   	}
>   
> +	data->call_ready = g_at_chat_register(data->aux, "Call Ready",
> +						call_ready_notify, false,
> +						modem, NULL);
> +	if (!data->call_ready) {
> +		close_serial(modem);
> +		return -ENOTTY;
> +	}
> +
>   	g_at_chat_set_slave(data->modem, data->aux);
>   
>   	g_at_chat_send(data->modem, "ATE0; &C0; +CMEE=1", none_prefix, NULL,
> 

Applied, thanks.

Regards,
-Denis