atmodem: fix detection of ATD*99 for non-muxed serial ports

Commit Message

Martin Hundebøll April 25, 2024, 11:57 a.m. UTC

The gprs context probe function has logic to detect whether CGDATA is
supported by the modem, or if ATD*99 should be used instead. However, it
seems like this logic was wrongly placed after registering for CGEV
notification, which did an early return in case the passed chat did not
have a slave.

Thus the ATD*99 detection was skipped for USB modems using separate
"virtual" serial ports for command and data channels (i.e. ttyUSB0 for
AT and ttyUSB1 for PPP).

Fix USB modem case by moving the check for a slave chat (and the
registraion for CGEV notifcations) to after the CGDATA check, which lets
the ATD*99 check run uncodionally.
---
 drivers/atmodem/gprs-context.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

[e-mail] April 25, 2024, 8:20 p.m. UTC | #1

Hello:

This patch was applied to ofono.git (master)
by Denis Kenzior <[e-mail]>:

On Thu, 25 Apr 2024 13:57:43 +0200 you wrote:
> The gprs context probe function has logic to detect whether CGDATA is
> supported by the modem, or if ATD*99 should be used instead. However, it
> seems like this logic was wrongly placed after registering for CGEV
> notification, which did an early return in case the passed chat did not
> have a slave.
> 
> Thus the ATD*99 detection was skipped for USB modems using separate
> "virtual" serial ports for command and data channels (i.e. ttyUSB0 for
> AT and ttyUSB1 for PPP).
> 
> [...]

Here is the summary with links:
  - atmodem: fix detection of ATD*99 for non-muxed serial ports
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=d42aad3e9553

You are awesome, thank you!

Patch

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index d92b6392..bfe6ae56 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -458,10 +458,6 @@  static int at_gprs_context_probe(struct ofono_gprs_context *gc,
 
 	ofono_gprs_context_set_data(gc, gcd);
 
-	chat = g_at_chat_get_slave(gcd->chat);
-	if (chat == NULL)
-		return 0;
-
 	switch (vendor) {
 	case OFONO_VENDOR_SIMCOM_SIM900:
 		gcd->use_atd99 = FALSE;
@@ -471,7 +467,10 @@  static int at_gprs_context_probe(struct ofono_gprs_context *gc,
 						at_cgdata_test_cb, gc, NULL);
 	}
 
-	g_at_chat_register(chat, "+CGEV:", cgev_notify, FALSE, gc, NULL);
+	chat = g_at_chat_get_slave(gcd->chat);
+	if (chat)
+		g_at_chat_register(chat, "+CGEV:", cgev_notify, FALSE, gc,
+									NULL);
 
 	return 0;
 }