[PATCH 0/7] Add quectel EC21 in serial mode

[PATCH 0/7] Add quectel EC21 in serial mode

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

This patchset adds support for the quectel EC21 LTE modem connected
through its serial port.

Lars Poeschel (7):
  quectel: Add Quectel EC21 to known serial modems
  quectel: use lte atom on EC21
  quectel: Query the model before setting up the mux
  quectel: EC21 needs aux channel to be the first mux channel
  quectel: EC21 does not understand AT+QIURC
  voicecall: Quectel modem do not understand AT+CNAP
  quectel: EC21 add ussd with atmodem driver

 drivers/atmodem/voicecall.c |   4 +-
 plugins/quectel.c           | 153 +++++++++++++++++++++++-------------
 2 files changed, 101 insertions(+), 56 deletions(-)

-- 
2.26.2

[PATCH 1/7] quectel: Add Quectel EC21 to known serial modems

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

This adds the Quectel EC21 to the known modems of the quectel driver and
therefore allows to use it with its serial interface.
---
 plugins/quectel.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 5d3ad470..1a49a5b3 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -64,7 +64,7 @@ static const char *cpin_prefix[] = { "+CPIN:", NULL };
 static const char *cbc_prefix[] = { "+CBC:", NULL };
 static const char *qinistat_prefix[] = { "+QINISTAT:", NULL };
 static const char *cgmm_prefix[] = { "UC15", "Quectel_M95", "Quectel_MC60",
-					NULL };
+					"EC21", NULL };
 static const char *none_prefix[] = { NULL };
 
 static const uint8_t gsm0710_terminate[] = {
@@ -83,6 +83,7 @@ enum quectel_model {
 	QUECTEL_UC15,
 	QUECTEL_M95,
 	QUECTEL_MC60,
+	QUECTEL_EC21,
 };
 
 struct quectel_data {
@@ -512,6 +513,7 @@ static void dbus_hw_enable(struct ofono_modem *modem)
 
 	switch (data->model) {
 	case QUECTEL_UC15:
+	case QUECTEL_EC21:
 		g_at_chat_register(data->aux, "+QIND",  qind_notify, FALSE, hw,
 					NULL);
 		break;
@@ -556,6 +558,7 @@ static void qinistat_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	switch (data->model) {
 	case QUECTEL_UC15:
+	case QUECTEL_EC21:
 		/* UC15 uses a bitmap of 1 + 2 + 4 = 7 */
 		ready = 7;
 		break;
@@ -788,6 +791,10 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 		DBG("%p model MC60", modem);
 		data->vendor = OFONO_VENDOR_QUECTEL_SERIAL;
 		data->model = QUECTEL_MC60;
+	} else if (strcmp(model, "EC21") == 0) {
+		DBG("%p model EC21", modem);
+		data->vendor = OFONO_VENDOR_QUECTEL;
+		data->model = QUECTEL_EC21;
 	} else {
 		ofono_warn("%p unknown model: '%s'", modem, model);
 		data->vendor = OFONO_VENDOR_QUECTEL;
-- 
2.26.2

[PATCH 2/7] quectel: use lte atom on EC21

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

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

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 1a49a5b3..0f6896b1 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -1263,6 +1263,9 @@ static void quectel_post_sim(struct ofono_modem *modem)
 	ofono_sms_create(modem, data->vendor, "atmodem", data->aux);
 	ofono_phonebook_create(modem, data->vendor, "atmodem", data->aux);
 	ofono_call_volume_create(modem, data->vendor, "atmodem", data->aux);
+
+	if (data->model == QUECTEL_EC21)
+		ofono_lte_create(modem, data->vendor, "atmodem", data->aux);
 }
 
 static void quectel_post_online(struct ofono_modem *modem)
-- 
2.26.2

[PATCH 3/7] quectel: Query the model before setting up the mux

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

This is a change for the EC21. It will require specific handling before
and right after setting up the mux. So this change prepares this. It
queries the modem model before the mux (CMUX) is activated.
---
 plugins/quectel.c | 86 +++++++++++++++++++++++------------------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 0f6896b1..1d312c45 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -765,46 +765,6 @@ 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);
-	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;
-		data->model = QUECTEL_UC15;
-	} else if (strcmp(model, "Quectel_M95") == 0) {
-		DBG("%p model M95", modem);
-		data->vendor = OFONO_VENDOR_QUECTEL_SERIAL;
-		data->model = QUECTEL_M95;
-	} else if (strcmp(model, "Quectel_MC60") == 0) {
-		DBG("%p model MC60", modem);
-		data->vendor = OFONO_VENDOR_QUECTEL_SERIAL;
-		data->model = QUECTEL_MC60;
-	} else if (strcmp(model, "EC21") == 0) {
-		DBG("%p model EC21", modem);
-		data->vendor = OFONO_VENDOR_QUECTEL;
-		data->model = QUECTEL_EC21;
-	} else {
-		ofono_warn("%p unknown model: '%s'", modem, model);
-		data->vendor = OFONO_VENDOR_QUECTEL;
-		data->model = QUECTEL_UNKNOWN;
-	}
-
-	g_at_chat_send(data->aux, "AT+CFUN?", cfun_prefix, cfun_query, modem,
-			NULL);
-}
-
 static void setup_aux(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
@@ -814,7 +774,7 @@ static void setup_aux(struct ofono_modem *modem)
 	g_at_chat_set_slave(data->modem, data->aux);
 	g_at_chat_send(data->aux, "ATE0; &C0; +CMEE=1; +QIURC=0", none_prefix,
 			NULL, NULL, NULL);
-	g_at_chat_send(data->aux, "AT+CGMM", cgmm_prefix, cgmm_cb, modem,
+	g_at_chat_send(data->aux, "AT+CFUN?", cfun_prefix, cfun_query, modem,
 			NULL);
 }
 
@@ -1034,17 +994,57 @@ static void cmux_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	close_serial(modem);
 }
 
-static void ate_cb(int ok, GAtResult *result, void *user_data)
+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);
+	const char *model;
 
-	DBG("%p", modem);
+	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;
+		data->model = QUECTEL_UC15;
+	} else if (strcmp(model, "Quectel_M95") == 0) {
+		DBG("%p model M95", modem);
+		data->vendor = OFONO_VENDOR_QUECTEL_SERIAL;
+		data->model = QUECTEL_M95;
+	} else if (strcmp(model, "Quectel_MC60") == 0) {
+		DBG("%p model MC60", modem);
+		data->vendor = OFONO_VENDOR_QUECTEL_SERIAL;
+		data->model = QUECTEL_MC60;
+	} else if (strcmp(model, "EC21") == 0) {
+		DBG("%p model EC21", modem);
+		data->vendor = OFONO_VENDOR_QUECTEL;
+		data->model = QUECTEL_EC21;
+	} else {
+		ofono_warn("%p unknown model: '%s'", modem, model);
+		data->vendor = OFONO_VENDOR_QUECTEL;
+		data->model = QUECTEL_UNKNOWN;
+	}
 
 	g_at_chat_send(data->uart, "AT+CMUX=0,0,5,127,10,3,30,10,2", NULL,
 			cmux_cb, modem, NULL);
 }
 
+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_send(data->uart, "AT+CGMM", cgmm_prefix, cgmm_cb, modem,
+			NULL);
+}
+
 static void init_cmd_cb(gboolean ok, GAtResult *result, void *user_data)
 {
 	struct ofono_modem *modem = user_data;
-- 
2.26.2

[PATCH 4/7] quectel: EC21 needs aux channel to be the first mux channel

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

The Quectel EC21 does only work correctly, if the mux channel used for
aux is the first mux channel. It does only put it's URC messages in the
first mux channel, so this has to be the aux channel in our case.
---
 plugins/quectel.c | 51 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 1d312c45..9ff75516 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -847,18 +847,38 @@ static void cmux_gatmux(struct ofono_modem *modem)
 
 	g_at_mux_start(data->mux);
 
-	data->modem = create_chat(modem, "Modem: ");
-	if (!data->modem) {
-		ofono_error("failed to create modem channel");
-		close_serial(modem);
-		return;
-	}
+	if (data->model == QUECTEL_EC21) {
+		data->aux = create_chat(modem, "Aux: ");
 
-	data->aux = create_chat(modem, "Aux: ");
-	if (!data->aux) {
-		ofono_error("failed to create aux channel");
-		close_serial(modem);
-		return;
+		if (!data->aux) {
+			ofono_error("failed to create aux channel");
+			close_serial(modem);
+			return;
+		}
+
+		data->modem = create_chat(modem, "Modem: ");
+
+		if (!data->modem) {
+			ofono_error("failed to create modem channel");
+			close_serial(modem);
+			return;
+		}
+	} else {
+		data->modem = create_chat(modem, "Modem: ");
+
+		if (!data->modem) {
+			ofono_error("failed to create modem channel");
+			close_serial(modem);
+			return;
+		}
+
+		data->aux = create_chat(modem, "Aux: ");
+
+		if (!data->aux) {
+			ofono_error("failed to create aux channel");
+			close_serial(modem);
+			return;
+		}
 	}
 
 	setup_aux(modem);
@@ -951,8 +971,13 @@ static void cmux_ngsm(struct ofono_modem *modem)
 	 * 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, "Modem", "/dev/gsmtty1");
-	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
+	if (data->model == QUECTEL_EC21) {
+		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty2");
+		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty1");
+	} else {
+		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
+		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
+	}
 
 	/* wait for gsmtty devices to appear */
 	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
-- 
2.26.2

[PATCH 5/7] quectel: EC21 does not understand AT+QIURC

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

Because the Quectel EC21 does not understand the AT+QIURC command, we
leave that out during initialisation.
---
 plugins/quectel.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 9ff75516..574de44e 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -772,8 +772,14 @@ static void setup_aux(struct ofono_modem *modem)
 	DBG("%p", modem);
 
 	g_at_chat_set_slave(data->modem, data->aux);
-	g_at_chat_send(data->aux, "ATE0; &C0; +CMEE=1; +QIURC=0", none_prefix,
-			NULL, NULL, NULL);
+
+	if (data->model == QUECTEL_EC21)
+		g_at_chat_send(data->aux, "ATE0; &C0; +CMEE=1", none_prefix,
+				NULL, NULL, NULL);
+	else
+		g_at_chat_send(data->aux, "ATE0; &C0; +CMEE=1; +QIURC=0",
+				none_prefix, NULL, NULL, NULL);
+
 	g_at_chat_send(data->aux, "AT+CFUN?", cfun_prefix, cfun_query, modem,
 			NULL);
 }
-- 
2.26.2

[PATCH 6/7] voicecall: Quectel modem do not understand AT+CNAP

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

---
 drivers/atmodem/voicecall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/atmodem/voicecall.c b/drivers/atmodem/voicecall.c
index 7ab6567f..e7f24b60 100644
--- a/drivers/atmodem/voicecall.c
+++ b/drivers/atmodem/voicecall.c
@@ -1113,7 +1113,9 @@ static int at_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
 	g_at_chat_send(vd->chat, "AT+CRC=1", NULL, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+CLIP=1", NULL, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+CDIP=1", NULL, NULL, NULL, NULL);
-	g_at_chat_send(vd->chat, "AT+CNAP=1", NULL, NULL, NULL, NULL);
+
+	if (vd->vendor != OFONO_VENDOR_QUECTEL)
+		g_at_chat_send(vd->chat, "AT+CNAP=1", NULL, NULL, NULL, NULL);
 
 	switch (vd->vendor) {
 	case OFONO_VENDOR_QUALCOMM_MSM:
-- 
2.26.2

[PATCH 7/7] quectel: EC21 add ussd with atmodem driver

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

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

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 574de44e..a8fbc75a 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -1295,8 +1295,10 @@ static void quectel_post_sim(struct ofono_modem *modem)
 	ofono_phonebook_create(modem, data->vendor, "atmodem", data->aux);
 	ofono_call_volume_create(modem, data->vendor, "atmodem", data->aux);
 
-	if (data->model == QUECTEL_EC21)
+	if (data->model == QUECTEL_EC21) {
+		ofono_ussd_create(modem, data->vendor, "atmodem", data->aux);
 		ofono_lte_create(modem, data->vendor, "atmodem", data->aux);
+	}
 }
 
 static void quectel_post_online(struct ofono_modem *modem)
-- 
2.26.2

Re: [PATCH 0/7] Add quectel EC21 in serial mode

[Denis Kenzior]

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

Hi Lars,

On 5/26/20 5:16 AM, poeschel(a)lemonage.de wrote:
> From: Lars Poeschel <poeschel@lemonage.de>
> 
> This patchset adds support for the quectel EC21 LTE modem connected
> through its serial port.
> 
> Lars Poeschel (7):
>    quectel: Add Quectel EC21 to known serial modems
>    quectel: use lte atom on EC21
>    quectel: Query the model before setting up the mux
>    quectel: EC21 needs aux channel to be the first mux channel
>    quectel: EC21 does not understand AT+QIURC
>    voicecall: Quectel modem do not understand AT+CNAP
>    quectel: EC21 add ussd with atmodem driver
> 
>   drivers/atmodem/voicecall.c |   4 +-
>   plugins/quectel.c           | 153 +++++++++++++++++++++++-------------
>   2 files changed, 101 insertions(+), 56 deletions(-)
> 


So I went ahead and applied everything except patch 4.  See my comments 
in the reply to that patch....

Regards,
-Denis

Re: [PATCH 4/7] quectel: EC21 needs aux channel to be the first mux channel

[Denis Kenzior]

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

Hi Lars,

On 5/26/20 5:16 AM, poeschel(a)lemonage.de wrote:
> From: Lars Poeschel <poeschel@lemonage.de>
> 
> The Quectel EC21 does only work correctly, if the mux channel used for
> aux is the first mux channel. It does only put it's URC messages in the
> first mux channel, so this has to be the aux channel in our case.
> ---
>   plugins/quectel.c | 51 +++++++++++++++++++++++++++++++++++------------
>   1 file changed, 38 insertions(+), 13 deletions(-)
> 
> diff --git a/plugins/quectel.c b/plugins/quectel.c
> index 1d312c45..9ff75516 100644
> --- a/plugins/quectel.c
> +++ b/plugins/quectel.c
> @@ -847,18 +847,38 @@ static void cmux_gatmux(struct ofono_modem *modem)
>   
>   	g_at_mux_start(data->mux);
>   
> -	data->modem = create_chat(modem, "Modem: ");
> -	if (!data->modem) {
> -		ofono_error("failed to create modem channel");
> -		close_serial(modem);
> -		return;
> -	}
> +	if (data->model == QUECTEL_EC21) {
> +		data->aux = create_chat(modem, "Aux: ");
>   
> -	data->aux = create_chat(modem, "Aux: ");
> -	if (!data->aux) {
> -		ofono_error("failed to create aux channel");
> -		close_serial(modem);
> -		return;
> +		if (!data->aux) {
> +			ofono_error("failed to create aux channel");
> +			close_serial(modem);
> +			return;
> +		}
> +
> +		data->modem = create_chat(modem, "Modem: ");
> +
> +		if (!data->modem) {
> +			ofono_error("failed to create modem channel");
> +			close_serial(modem);
> +			return;
> +		}
> +	} else {
> +		data->modem = create_chat(modem, "Modem: ");
> +
> +		if (!data->modem) {
> +			ofono_error("failed to create modem channel");
> +			close_serial(modem);
> +			return;
> +		}
> +
> +		data->aux = create_chat(modem, "Aux: ");
> +
> +		if (!data->aux) {
> +			ofono_error("failed to create aux channel");
> +			close_serial(modem);
> +			return;
> +		}
>   	}

Can we be smarter about this and just store the per-model creation 
sequence as an array or something?  Then have a loop that calls 
create_chat from the array info?  The proposed copy-pasting approach is 
not maintainable long term.

>   
>   	setup_aux(modem);
> @@ -951,8 +971,13 @@ static void cmux_ngsm(struct ofono_modem *modem)
>   	 * 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, "Modem", "/dev/gsmtty1");
> -	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
> +	if (data->model == QUECTEL_EC21) {
> +		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty2");
> +		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty1");
> +	} else {
> +		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
> +		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
> +	}

Doesn't this break the logic in mux_ready_cb, particularly the 'check if 
the last virtual gsm tty's are created'

>   
>   	/* wait for gsmtty devices to appear */
>   	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
> 

Regards,
-Denis

Re: [PATCH 4/7] quectel: EC21 needs aux channel to be the first mux channel

[Lars Poeschel]

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

On Thu, May 21, 2020 at 08:32:07AM -0500, Denis Kenzior wrote:
> Hi Lars,
> 
> On 5/26/20 5:16 AM, poeschel(a)lemonage.de wrote:
> > From: Lars Poeschel <poeschel@lemonage.de>
> > 
> > The Quectel EC21 does only work correctly, if the mux channel used for
> > aux is the first mux channel. It does only put it's URC messages in the
> > first mux channel, so this has to be the aux channel in our case.
> > ---
> >   plugins/quectel.c | 51 +++++++++++++++++++++++++++++++++++------------
> >   1 file changed, 38 insertions(+), 13 deletions(-)
> > 
> > diff --git a/plugins/quectel.c b/plugins/quectel.c
> > index 1d312c45..9ff75516 100644
> > --- a/plugins/quectel.c
> > +++ b/plugins/quectel.c
> > @@ -847,18 +847,38 @@ static void cmux_gatmux(struct ofono_modem *modem)
> >   	g_at_mux_start(data->mux);
> > -	data->modem = create_chat(modem, "Modem: ");
> > -	if (!data->modem) {
> > -		ofono_error("failed to create modem channel");
> > -		close_serial(modem);
> > -		return;
> > -	}
> > +	if (data->model == QUECTEL_EC21) {
> > +		data->aux = create_chat(modem, "Aux: ");
> > -	data->aux = create_chat(modem, "Aux: ");
> > -	if (!data->aux) {
> > -		ofono_error("failed to create aux channel");
> > -		close_serial(modem);
> > -		return;
> > +		if (!data->aux) {
> > +			ofono_error("failed to create aux channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> > +
> > +		data->modem = create_chat(modem, "Modem: ");
> > +
> > +		if (!data->modem) {
> > +			ofono_error("failed to create modem channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> > +	} else {
> > +		data->modem = create_chat(modem, "Modem: ");
> > +
> > +		if (!data->modem) {
> > +			ofono_error("failed to create modem channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> > +
> > +		data->aux = create_chat(modem, "Aux: ");
> > +
> > +		if (!data->aux) {
> > +			ofono_error("failed to create aux channel");
> > +			close_serial(modem);
> > +			return;
> > +		}
> >   	}
> 
> Can we be smarter about this and just store the per-model creation sequence
> as an array or something?  Then have a loop that calls create_chat from the
> array info?  The proposed copy-pasting approach is not maintainable long
> term.

I will post a follow-up with a proposed solution tomorrow.

> >   	setup_aux(modem);
> > @@ -951,8 +971,13 @@ static void cmux_ngsm(struct ofono_modem *modem)
> >   	 * 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, "Modem", "/dev/gsmtty1");
> > -	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
> > +	if (data->model == QUECTEL_EC21) {
> > +		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty2");
> > +		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty1");
> > +	} else {
> > +		ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
> > +		ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
> > +	}
> 
> Doesn't this break the logic in mux_ready_cb, particularly the 'check if the
> last virtual gsm tty's are created'

Well, I guess, yes. I missed indeed missed that. The follow-up will
contain a fix for this as well.

> >   	/* wait for gsmtty devices to appear */
> >   	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
> > 
> 
> Regards,
> -Denis

Thank you for reviewing and applying the other patches! :-)

Lars

[PATCH v2] quectel: EC21 needs aux channel to be the first mux channel

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

The Quectel EC21 does only work correctly, if the mux channel used for
aux is the first mux channel. It does only put it's URC messages in the
first mux channel, so this has to be the aux channel in our case.
To be flexible on the mux order we introduce an array here, that
contains the initialization data in it's needed order and is then simply
applied by a for-loop. Initialization of this array is done after we
queried the modem model.
---
 plugins/quectel.c | 72 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 14 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 043d39f9..1cad35d6 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -78,6 +78,21 @@ static const uint8_t gsm0710_terminate[] = {
 	0xf9, /* close flag */
 };
 
+enum mux_type {
+	QUECTEL_MUX_TYPE_AUX = 0,
+	QUECTEL_MUX_TYPE_MODEM,
+	QUECTEL_MUX_TYPE_MAX,
+};
+
+struct mux_initialization_data {
+	enum mux_type mux_type;
+	char *chat_debug;
+	const char *n_gsm_key;
+	const char *n_gsm_value;
+};
+
+static struct mux_initialization_data mux_order[QUECTEL_MUX_TYPE_MAX];
+
 enum quectel_model {
 	QUECTEL_UNKNOWN,
 	QUECTEL_UC15,
@@ -838,6 +853,7 @@ static GAtChat *create_chat(struct ofono_modem *modem, char *debug)
 static void cmux_gatmux(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
+	GAtChat *chat;
 
 	DBG("%p", modem);
 
@@ -853,18 +869,21 @@ static void cmux_gatmux(struct ofono_modem *modem)
 
 	g_at_mux_start(data->mux);
 
-	data->modem = create_chat(modem, "Modem: ");
-	if (!data->modem) {
-		ofono_error("failed to create modem channel");
-		close_serial(modem);
-		return;
-	}
+	for (int i = 0; i < QUECTEL_MUX_TYPE_MAX; i++) {
+		chat = create_chat(modem, mux_order[i].chat_debug);
+
+		if (!chat) {
+			ofono_error("failed to create %schannel",
+					mux_order[i].chat_debug);
+			close_serial(modem);
+			return;
+		}
+
+		if (mux_order[i].mux_type == QUECTEL_MUX_TYPE_AUX)
+			data->aux = chat;
+		else
+			data->modem = chat;
 
-	data->aux = create_chat(modem, "Aux: ");
-	if (!data->aux) {
-		ofono_error("failed to create aux channel");
-		close_serial(modem);
-		return;
 	}
 
 	setup_aux(modem);
@@ -880,7 +899,9 @@ static void mux_ready_cb(struct l_timeout *timeout, void *user_data)
 	DBG("%p", modem);
 
 	/* check if the last (and thus all) virtual gsm tty's are created */
-	ret = stat(ofono_modem_get_string(modem, "Modem"), &st);
+	ret = stat(ofono_modem_get_string(modem,
+				mux_order[QUECTEL_MUX_TYPE_MAX - 1].n_gsm_key),
+				&st);
 	if (ret < 0) {
 		if (data->mux_ready_count++ < 5) {
 			/* not ready yet; try again in 100 ms*/
@@ -957,8 +978,10 @@ static void cmux_ngsm(struct ofono_modem *modem)
 	 * 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, "Modem", "/dev/gsmtty1");
-	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
+	for (int i = 0; i < QUECTEL_MUX_TYPE_MAX; i++) {
+		ofono_modem_set_string(modem, mux_order[i].n_gsm_key,
+					mux_order[i].n_gsm_value);
+	}
 
 	/* wait for gsmtty devices to appear */
 	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
@@ -1014,6 +1037,17 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 		return;
 	}
 
+	mux_order[0] = (struct mux_initialization_data)
+			{ QUECTEL_MUX_TYPE_MODEM,
+			"Modem: ",
+			"Modem",
+			"/dev/gsmtty1"};
+	mux_order[1] = (struct mux_initialization_data)
+			{ QUECTEL_MUX_TYPE_AUX,
+			"Aux: ",
+			"Aux",
+			"/dev/gsmtty2"};
+
 	if (strcmp(model, "UC15") == 0) {
 		DBG("%p model UC15", modem);
 		data->vendor = OFONO_VENDOR_QUECTEL;
@@ -1030,6 +1064,16 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 		DBG("%p model EC21", modem);
 		data->vendor = OFONO_VENDOR_QUECTEL;
 		data->model = QUECTEL_EC21;
+		mux_order[0] = (struct mux_initialization_data)
+				{ QUECTEL_MUX_TYPE_AUX,
+				"Aux: ",
+				"Aux",
+				"/dev/gsmtty1"};
+		mux_order[1] = (struct mux_initialization_data)
+				{ QUECTEL_MUX_TYPE_MODEM,
+				"Modem: ",
+				"Modem",
+				"/dev/gsmtty2"};
 	} else {
 		ofono_warn("%p unknown model: '%s'", modem, model);
 		data->vendor = OFONO_VENDOR_QUECTEL;
-- 
2.26.2

Re: [PATCH v2] quectel: EC21 needs aux channel to be the first mux channel

[Denis Kenzior]

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

Hi Lars,

On 5/28/20 4:32 AM, poeschel(a)lemonage.de wrote:
> From: Lars Poeschel <poeschel@lemonage.de>
> 
> The Quectel EC21 does only work correctly, if the mux channel used for
> aux is the first mux channel. It does only put it's URC messages in the
> first mux channel, so this has to be the aux channel in our case.
> To be flexible on the mux order we introduce an array here, that
> contains the initialization data in it's needed order and is then simply
> applied by a for-loop. Initialization of this array is done after we
> queried the modem model.
> ---
>   plugins/quectel.c | 72 ++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 58 insertions(+), 14 deletions(-)
> 

So this looks mostly reasonable, but see below:

> diff --git a/plugins/quectel.c b/plugins/quectel.c
> index 043d39f9..1cad35d6 100644
> --- a/plugins/quectel.c
> +++ b/plugins/quectel.c
> @@ -78,6 +78,21 @@ static const uint8_t gsm0710_terminate[] = {
>   	0xf9, /* close flag */
>   };
>   
> +enum mux_type {
> +	QUECTEL_MUX_TYPE_AUX = 0,
> +	QUECTEL_MUX_TYPE_MODEM,
> +	QUECTEL_MUX_TYPE_MAX,
> +};
> +
> +struct mux_initialization_data {
> +	enum mux_type mux_type;
> +	char *chat_debug;
> +	const char *n_gsm_key;
> +	const char *n_gsm_value;
> +};
> +
> +static struct mux_initialization_data mux_order[QUECTEL_MUX_TYPE_MAX];
> +

So the issue with this is that this driver now no-longer supports 
multiple modems of the same type active simultaneously (since all 
instances would be trying to write / read from this location).

A better approach would be to use two such variables, i.e. 
mux_order_ec21 and mux_order_default and then either store a pointer to 
the one to use inside quectel_data, or programmatically by looking up 
based on the quectel_data::model.

Alternatively, storing mux_order in quectel_data itself is also an option.

>   enum quectel_model {
>   	QUECTEL_UNKNOWN,
>   	QUECTEL_UC15,
> @@ -1014,6 +1037,17 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
>   		return;
>   	}
>   
> +	mux_order[0] = (struct mux_initialization_data)
> +			{ QUECTEL_MUX_TYPE_MODEM,
> +			"Modem: ",
> +			"Modem",
> +			"/dev/gsmtty1"};
> +	mux_order[1] = (struct mux_initialization_data)
> +			{ QUECTEL_MUX_TYPE_AUX,
> +			"Aux: ",
> +			"Aux",
> +			"/dev/gsmtty2"};
> +

This would then move into the static initializer above...

>   	if (strcmp(model, "UC15") == 0) {
>   		DBG("%p model UC15", modem);
>   		data->vendor = OFONO_VENDOR_QUECTEL;
> @@ -1030,6 +1064,16 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
>   		DBG("%p model EC21", modem);
>   		data->vendor = OFONO_VENDOR_QUECTEL;
>   		data->model = QUECTEL_EC21;
> +		mux_order[0] = (struct mux_initialization_data)
> +				{ QUECTEL_MUX_TYPE_AUX,
> +				"Aux: ",
> +				"Aux",
> +				"/dev/gsmtty1"};
> +		mux_order[1] = (struct mux_initialization_data)
> +				{ QUECTEL_MUX_TYPE_MODEM,
> +				"Modem: ",
> +				"Modem",
> +				"/dev/gsmtty2"};
>   	} else {
>   		ofono_warn("%p unknown model: '%s'", modem, model);
>   		data->vendor = OFONO_VENDOR_QUECTEL;
> 

Regards,
-Denis

[PATCH v3] quectel: EC21 needs aux channel to be the first mux channel

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

The Quectel EC21 does only work correctly, if the mux channel used for
aux is the first mux channel. It does only put it's URC messages in the
first mux channel, so this has to be the aux channel in our case.
To be flexible on the mux order we introduce two arrays here, that then
contain the initialization data in their needed order.
Initialization data is then applied by for-looping over this array.
---
 plugins/quectel.c | 61 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 043d39f9..cb15e147 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -78,6 +78,27 @@ static const uint8_t gsm0710_terminate[] = {
 	0xf9, /* close flag */
 };
 
+enum mux_type {
+	QUECTEL_MUX_TYPE_AUX = 0,
+	QUECTEL_MUX_TYPE_MODEM,
+	QUECTEL_MUX_TYPE_MAX,
+};
+
+struct mux_initialization_data {
+	enum mux_type mux_type;
+	char *chat_debug;
+	const char *n_gsm_key;
+	const char *n_gsm_value;
+};
+
+static const struct mux_initialization_data mux_order_default[] = {
+		{ QUECTEL_MUX_TYPE_MODEM, "Modem: ", "Modem", "/dev/gsmtty1"},
+		{ QUECTEL_MUX_TYPE_AUX, "Aux: ", "Aux", "/dev/gsmtty2"} };
+
+static const struct mux_initialization_data mux_order_ec21[] = {
+		{ QUECTEL_MUX_TYPE_AUX, "Aux: ", "Aux", "/dev/gsmtty1"},
+		{ QUECTEL_MUX_TYPE_MODEM, "Modem: ", "Modem", "/dev/gsmtty2"} };
+
 enum quectel_model {
 	QUECTEL_UNKNOWN,
 	QUECTEL_UC15,
@@ -106,6 +127,7 @@ struct quectel_data {
 	struct l_timeout *init_timeout;
 	size_t init_count;
 	guint init_cmd;
+	const struct mux_initialization_data *mux_order;
 };
 
 struct dbus_hw {
@@ -838,6 +860,7 @@ static GAtChat *create_chat(struct ofono_modem *modem, char *debug)
 static void cmux_gatmux(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
+	GAtChat *chat;
 
 	DBG("%p", modem);
 
@@ -853,18 +876,21 @@ static void cmux_gatmux(struct ofono_modem *modem)
 
 	g_at_mux_start(data->mux);
 
-	data->modem = create_chat(modem, "Modem: ");
-	if (!data->modem) {
-		ofono_error("failed to create modem channel");
-		close_serial(modem);
-		return;
-	}
+	for (int i = 0; i < QUECTEL_MUX_TYPE_MAX; i++) {
+		chat = create_chat(modem, data->mux_order[i].chat_debug);
+
+		if (!chat) {
+			ofono_error("failed to create %schannel",
+					data->mux_order[i].chat_debug);
+			close_serial(modem);
+			return;
+		}
+
+		if (data->mux_order[i].mux_type == QUECTEL_MUX_TYPE_AUX)
+			data->aux = chat;
+		else
+			data->modem = chat;
 
-	data->aux = create_chat(modem, "Aux: ");
-	if (!data->aux) {
-		ofono_error("failed to create aux channel");
-		close_serial(modem);
-		return;
 	}
 
 	setup_aux(modem);
@@ -880,7 +906,9 @@ static void mux_ready_cb(struct l_timeout *timeout, void *user_data)
 	DBG("%p", modem);
 
 	/* check if the last (and thus all) virtual gsm tty's are created */
-	ret = stat(ofono_modem_get_string(modem, "Modem"), &st);
+	ret = stat(ofono_modem_get_string(modem,
+			data->mux_order[QUECTEL_MUX_TYPE_MAX - 1].n_gsm_key),
+			&st);
 	if (ret < 0) {
 		if (data->mux_ready_count++ < 5) {
 			/* not ready yet; try again in 100 ms*/
@@ -957,8 +985,10 @@ static void cmux_ngsm(struct ofono_modem *modem)
 	 * 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, "Modem", "/dev/gsmtty1");
-	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
+	for (int i = 0; i < QUECTEL_MUX_TYPE_MAX; i++) {
+		ofono_modem_set_string(modem, data->mux_order[i].n_gsm_key,
+					data->mux_order[i].n_gsm_value);
+	}
 
 	/* wait for gsmtty devices to appear */
 	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
@@ -1014,6 +1044,8 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 		return;
 	}
 
+	data->mux_order = mux_order_default;
+
 	if (strcmp(model, "UC15") == 0) {
 		DBG("%p model UC15", modem);
 		data->vendor = OFONO_VENDOR_QUECTEL;
@@ -1030,6 +1062,7 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 		DBG("%p model EC21", modem);
 		data->vendor = OFONO_VENDOR_QUECTEL;
 		data->model = QUECTEL_EC21;
+		data->mux_order = mux_order_ec21;
 	} else {
 		ofono_warn("%p unknown model: '%s'", modem, model);
 		data->vendor = OFONO_VENDOR_QUECTEL;
-- 
2.26.2

Re: [PATCH v3] quectel: EC21 needs aux channel to be the first mux channel

[Denis Kenzior]

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

Hi Lars,

On 5/29/20 7:43 AM, poeschel(a)lemonage.de wrote:
> From: Lars Poeschel <poeschel@lemonage.de>
> 
> The Quectel EC21 does only work correctly, if the mux channel used for
> aux is the first mux channel. It does only put it's URC messages in the
> first mux channel, so this has to be the aux channel in our case.
> To be flexible on the mux order we introduce two arrays here, that then
> contain the initialization data in their needed order.
> Initialization data is then applied by for-looping over this array.
> ---
>   plugins/quectel.c | 61 ++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 47 insertions(+), 14 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH v3] quectel: EC21 needs aux channel to be the first mux channel

[Lars Poeschel]

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

On Tue, May 26, 2020 at 10:22:46PM -0500, Denis Kenzior wrote:
> Hi Lars,
> 
> On 5/29/20 7:43 AM, poeschel(a)lemonage.de wrote:
> > From: Lars Poeschel <poeschel@lemonage.de>
> > 
> > The Quectel EC21 does only work correctly, if the mux channel used for
> > aux is the first mux channel. It does only put it's URC messages in the
> > first mux channel, so this has to be the aux channel in our case.
> > To be flexible on the mux order we introduce two arrays here, that then
> > contain the initialization data in their needed order.
> > Initialization data is then applied by for-looping over this array.
> > ---
> >   plugins/quectel.c | 61 ++++++++++++++++++++++++++++++++++++-----------
> >   1 file changed, 47 insertions(+), 14 deletions(-)
> > 
> 
> Applied, thanks.

Unfortunately I must come back to this issue.
I got hands on a few new EC21s here and guess what ?
The mux order is back to the original one again. This means, the aux
channel has to be the second channel.
So I did a bit of investigation why and when this happened. But
information is rare.
The modems I originally worked on and created the patch for have
Firmware EC21EFAR06A01M4G_BETA0318. (Reversed mux order)
The new ones do have version EC21EFAR06A03M4G. (original mux order)
I know that there was a version EC21EFAR02A02M4G that did not support
cmux at all.
Due to some Quectel Confidential Document in a firmware version
"R02A03" some bug was fixed in cmux, so cmux must be in there since
then.
The EC21EFAR06A01M4G_BETA0318 that I have is dated inbetween
EC21EFAR02A02M4G and this "R02A03".
The mux order must have changed between EC21EFAR06A01M4G_BETA0318 and
EC21EFAR06A03M4G.
I suspect (without knowing for sure) due to the beta-nature of my
firmware, that this is the only firmware with reversed mux order and
that they changed it after that and "R02A03" up until
EC21EFAR06A03M4G share the same original mux order. According to Quectel
the EC21EFAR06A01M4G_BETA0318 firmware I have here is not "out in the
wild". The modems I have here are the only ones with this firmware.

So my question is what's best to do now ?

I feel the best would be to revert this patch. I am very sorry for this.
New modems will work and I suspect old modems "out in the wild" will
work also. I don't care about supporting the few "BETA0318" modems I
have here.

Another way would be to leave this patch and implement some firmware
switch and use reversed mux order for the "BETA0318" only I have and use
normal original mux order for all other cases.

What do you think?

Thank you,
Lars

Re: [PATCH v3] quectel: EC21 needs aux channel to be the first mux channel

[Denis Kenzior]

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

Hi Lars,

> 
> Unfortunately I must come back to this issue.
> I got hands on a few new EC21s here and guess what ?
> The mux order is back to the original one again. This means, the aux
> channel has to be the second channel.
> So I did a bit of investigation why and when this happened. But
> information is rare.
> The modems I originally worked on and created the patch for have
> Firmware EC21EFAR06A01M4G_BETA0318. (Reversed mux order)
> The new ones do have version EC21EFAR06A03M4G. (original mux order)
> I know that there was a version EC21EFAR02A02M4G that did not support
> cmux at all.
> Due to some Quectel Confidential Document in a firmware version
> "R02A03" some bug was fixed in cmux, so cmux must be in there since
> then.
> The EC21EFAR06A01M4G_BETA0318 that I have is dated inbetween
> EC21EFAR02A02M4G and this "R02A03".
> The mux order must have changed between EC21EFAR06A01M4G_BETA0318 and
> EC21EFAR06A03M4G.
> I suspect (without knowing for sure) due to the beta-nature of my
> firmware, that this is the only firmware with reversed mux order and
> that they changed it after that and "R02A03" up until
> EC21EFAR06A03M4G share the same original mux order. According to Quectel
> the EC21EFAR06A01M4G_BETA0318 firmware I have here is not "out in the
> wild". The modems I have here are the only ones with this firmware.
> 
> So my question is what's best to do now ?
> 
> I feel the best would be to revert this patch. I am very sorry for this.
> New modems will work and I suspect old modems "out in the wild" will
> work also. I don't care about supporting the few "BETA0318" modems I
> have here.

I guess just send a revert patch with the reasons outlined in the commit 
description and I can take it.

> 
> Another way would be to leave this patch and implement some firmware
> switch and use reversed mux order for the "BETA0318" only I have and use
> normal original mux order for all other cases.

That would also be fine.

Regards,
-Denis

[PATCH] Revert "quectel: EC21 needs aux channel to be the first mux channel"

[poeschel]

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

From: Lars Poeschel <poeschel@lemonage.de>

This reverts commit 1868dbf2b3e5929a7081b03a8ff76d214fd38624.
Development for this was done on EC21 firmware version
EC21EFAR06A01M4G_BETA0318. It now turns out, that actual release
firmware versions for this modem again need the original mux order with
aux channel as the second mux channel. (We know for sure for firmware
version EC21EFAR06A03M4G.)
We do not know for sure when and for what firmware versions quectel did
the switch back on the mux order, but we suspect that the "BETA"
firmware is the only one with the reversed mux order. This "BETA"
firmware was only given out for development purposes and will not appear
"in the wild", so we revert the patch here and hope for the best.
---
 plugins/quectel.c | 61 +++++++++++------------------------------------
 1 file changed, 14 insertions(+), 47 deletions(-)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index cb15e147..043d39f9 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -78,27 +78,6 @@ static const uint8_t gsm0710_terminate[] = {
 	0xf9, /* close flag */
 };
 
-enum mux_type {
-	QUECTEL_MUX_TYPE_AUX = 0,
-	QUECTEL_MUX_TYPE_MODEM,
-	QUECTEL_MUX_TYPE_MAX,
-};
-
-struct mux_initialization_data {
-	enum mux_type mux_type;
-	char *chat_debug;
-	const char *n_gsm_key;
-	const char *n_gsm_value;
-};
-
-static const struct mux_initialization_data mux_order_default[] = {
-		{ QUECTEL_MUX_TYPE_MODEM, "Modem: ", "Modem", "/dev/gsmtty1"},
-		{ QUECTEL_MUX_TYPE_AUX, "Aux: ", "Aux", "/dev/gsmtty2"} };
-
-static const struct mux_initialization_data mux_order_ec21[] = {
-		{ QUECTEL_MUX_TYPE_AUX, "Aux: ", "Aux", "/dev/gsmtty1"},
-		{ QUECTEL_MUX_TYPE_MODEM, "Modem: ", "Modem", "/dev/gsmtty2"} };
-
 enum quectel_model {
 	QUECTEL_UNKNOWN,
 	QUECTEL_UC15,
@@ -127,7 +106,6 @@ struct quectel_data {
 	struct l_timeout *init_timeout;
 	size_t init_count;
 	guint init_cmd;
-	const struct mux_initialization_data *mux_order;
 };
 
 struct dbus_hw {
@@ -860,7 +838,6 @@ static GAtChat *create_chat(struct ofono_modem *modem, char *debug)
 static void cmux_gatmux(struct ofono_modem *modem)
 {
 	struct quectel_data *data = ofono_modem_get_data(modem);
-	GAtChat *chat;
 
 	DBG("%p", modem);
 
@@ -876,21 +853,18 @@ static void cmux_gatmux(struct ofono_modem *modem)
 
 	g_at_mux_start(data->mux);
 
-	for (int i = 0; i < QUECTEL_MUX_TYPE_MAX; i++) {
-		chat = create_chat(modem, data->mux_order[i].chat_debug);
-
-		if (!chat) {
-			ofono_error("failed to create %schannel",
-					data->mux_order[i].chat_debug);
-			close_serial(modem);
-			return;
-		}
-
-		if (data->mux_order[i].mux_type == QUECTEL_MUX_TYPE_AUX)
-			data->aux = chat;
-		else
-			data->modem = chat;
+	data->modem = create_chat(modem, "Modem: ");
+	if (!data->modem) {
+		ofono_error("failed to create modem channel");
+		close_serial(modem);
+		return;
+	}
 
+	data->aux = create_chat(modem, "Aux: ");
+	if (!data->aux) {
+		ofono_error("failed to create aux channel");
+		close_serial(modem);
+		return;
 	}
 
 	setup_aux(modem);
@@ -906,9 +880,7 @@ static void mux_ready_cb(struct l_timeout *timeout, void *user_data)
 	DBG("%p", modem);
 
 	/* check if the last (and thus all) virtual gsm tty's are created */
-	ret = stat(ofono_modem_get_string(modem,
-			data->mux_order[QUECTEL_MUX_TYPE_MAX - 1].n_gsm_key),
-			&st);
+	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*/
@@ -985,10 +957,8 @@ static void cmux_ngsm(struct ofono_modem *modem)
 	 * the kernel does not yet support mapping the underlying serial device
 	 * to its virtual gsm ttys, so hard-code gsmtty1 gsmtty2 for now
 	 */
-	for (int i = 0; i < QUECTEL_MUX_TYPE_MAX; i++) {
-		ofono_modem_set_string(modem, data->mux_order[i].n_gsm_key,
-					data->mux_order[i].n_gsm_value);
-	}
+	ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
+	ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
 
 	/* wait for gsmtty devices to appear */
 	if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
@@ -1044,8 +1014,6 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 		return;
 	}
 
-	data->mux_order = mux_order_default;
-
 	if (strcmp(model, "UC15") == 0) {
 		DBG("%p model UC15", modem);
 		data->vendor = OFONO_VENDOR_QUECTEL;
@@ -1062,7 +1030,6 @@ static void cgmm_cb(int ok, GAtResult *result, void *user_data)
 		DBG("%p model EC21", modem);
 		data->vendor = OFONO_VENDOR_QUECTEL;
 		data->model = QUECTEL_EC21;
-		data->mux_order = mux_order_ec21;
 	} else {
 		ofono_warn("%p unknown model: '%s'", modem, model);
 		data->vendor = OFONO_VENDOR_QUECTEL;
-- 
2.27.0

Re: [PATCH] Revert "quectel: EC21 needs aux channel to be the first mux channel"

[Denis Kenzior]

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

Hi Lars,

On 8/4/20 6:56 AM, poeschel(a)lemonage.de wrote:
> From: Lars Poeschel <poeschel@lemonage.de>
> 
> This reverts commit 1868dbf2b3e5929a7081b03a8ff76d214fd38624.
> Development for this was done on EC21 firmware version
> EC21EFAR06A01M4G_BETA0318. It now turns out, that actual release
> firmware versions for this modem again need the original mux order with
> aux channel as the second mux channel. (We know for sure for firmware
> version EC21EFAR06A03M4G.)
> We do not know for sure when and for what firmware versions quectel did
> the switch back on the mux order, but we suspect that the "BETA"
> firmware is the only one with the reversed mux order. This "BETA"
> firmware was only given out for development purposes and will not appear
> "in the wild", so we revert the patch here and hope for the best.
> ---
>   plugins/quectel.c | 61 +++++++++++------------------------------------
>   1 file changed, 14 insertions(+), 47 deletions(-)

Applied, thanks.

Regards,
-Denis