[PATCH 1/7] udevng/gobi: improve debug output

[PATCH 1/7] udevng/gobi: improve debug output

[Alexander Couzens]

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

Add info->sysattr and info->subsystem to the log message.
---
 plugins/udevng.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 5e74a7cc..47bc8910 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -204,8 +204,9 @@ static gboolean setup_gobi(struct modem_info *modem)
 	for (list = modem->devices; list; list = list->next) {
 		struct device_info *info = list->data;
 
-		DBG("%s %s %s %s", info->devnode, info->interface,
-						info->number, info->label);
+		DBG("%s %s %s %s %s %s", info->devnode, info->interface,
+						info->number, info->label,
+						info->sysattr, info->subsystem);
 
 		if (g_strcmp0(info->interface, "255/255/255") == 0) {
 			if (info->number == NULL)
-- 
2.12.2

[PATCH 2/7] udevng/gobi: use subsystem as first identification

[Alexander Couzens]

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

Using kernel 4.10 with systemd 232 on archlinux the detection of
a gobi 2000 doesn't detect the qmi/cdc-wdm interface.

The device is detected as follows:

[devnode interface number label sysattr subsystem]
/dev/cdc-wdm1 255/255/255 00 (null) (null) usbmisc
wwan1 255/255/255 00 (null) (null) net
/dev/ttyUSB5 255/255/255 01 (null) (null) tty
/dev/ttyUSB6 255/255/255 02 (null) (null) tty
/dev/ttyUSB7 255/255/255 03 (null) (null) tty
---
 plugins/udevng.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 47bc8910..e8b800c1 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -208,17 +208,19 @@ static gboolean setup_gobi(struct modem_info *modem)
 						info->number, info->label,
 						info->sysattr, info->subsystem);
 
-		if (g_strcmp0(info->interface, "255/255/255") == 0) {
-			if (info->number == NULL)
-				qmi = info->devnode;
-			else if (g_strcmp0(info->number, "00") == 0)
-				net = info->devnode;
-			else if (g_strcmp0(info->number, "01") == 0)
-				diag = info->devnode;
-			else if (g_strcmp0(info->number, "02") == 0)
-				mdm = info->devnode;
-			else if (g_strcmp0(info->number, "03") == 0)
-				gps = info->devnode;
+		if (g_strcmp0(info->subsystem, "usbmisc") == 0) /* cdc-wdm */
+			qmi = info->devnode;
+		else if (g_strcmp0(info->subsystem, "net") == 0) /* wwan */
+			net = info->devnode;
+		else if (g_strcmp0(info->subsystem, "tty") == 0) {
+			if (g_strcmp0(info->interface, "255/255/255") == 0) {
+				if (g_strcmp0(info->number, "01") == 0)
+					diag = info->devnode;
+				else if (g_strcmp0(info->number, "02") == 0)
+					mdm = info->devnode;
+				else if (g_strcmp0(info->number, "03") == 0)
+					gps = info->devnode;
+			}
 		}
 	}
 
-- 
2.12.2

Re: [PATCH 2/7] udevng/gobi: use subsystem as first identification

[Jonas Bonn]

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

On 05/01/2017 10:52 PM, Alexander Couzens wrote:
> Using kernel 4.10 with systemd 232 on archlinux the detection of
> a gobi 2000 doesn't detect the qmi/cdc-wdm interface.
>
> The device is detected as follows:
>
> [devnode interface number label sysattr subsystem]
> /dev/cdc-wdm1 255/255/255 00 (null) (null) usbmisc
> wwan1 255/255/255 00 (null) (null) net
> /dev/ttyUSB5 255/255/255 01 (null) (null) tty
> /dev/ttyUSB6 255/255/255 02 (null) (null) tty
> /dev/ttyUSB7 255/255/255 03 (null) (null) tty
> ---
>   plugins/udevng.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 47bc8910..e8b800c1 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -208,17 +208,19 @@ static gboolean setup_gobi(struct modem_info *modem)
>   						info->number, info->label,
>   						info->sysattr, info->subsystem);
>   
> -		if (g_strcmp0(info->interface, "255/255/255") == 0) {
> -			if (info->number == NULL)
> -				qmi = info->devnode;
> -			else if (g_strcmp0(info->number, "00") == 0)
> -				net = info->devnode;
> -			else if (g_strcmp0(info->number, "01") == 0)
> -				diag = info->devnode;
> -			else if (g_strcmp0(info->number, "02") == 0)
> -				mdm = info->devnode;
> -			else if (g_strcmp0(info->number, "03") == 0)
> -				gps = info->devnode;
> +		if (g_strcmp0(info->subsystem, "usbmisc") == 0) /* cdc-wdm */
> +			qmi = info->devnode;
> +		else if (g_strcmp0(info->subsystem, "net") == 0) /* wwan */
> +			net = info->devnode;
> +		else if (g_strcmp0(info->subsystem, "tty") == 0) {
> +			if (g_strcmp0(info->interface, "255/255/255") == 0) {
> +				if (g_strcmp0(info->number, "01") == 0)
> +					diag = info->devnode;
> +				else if (g_strcmp0(info->number, "02") == 0)
> +					mdm = info->devnode;
> +				else if (g_strcmp0(info->number, "03") == 0)
> +					gps = info->devnode;
> +			}

I also wondered about this code.  The original has a method of detecting 
the qmi and net interfaces that doesn't match the way things work with 
the qmiwwan driver.  I wonder if the proprietary gobi driver does things 
differently and wants the original incantation...???  Somebody with 
older hardware might have some insight here.

I suspect that we might want to separate the handling of the gobi and 
qmiwwan drivers:  setup_gobi for the 'gobi' driver and a new 
setup_qmiwwan function if the 'qmiwwan' driver is in use.

/Jonas

>   		}
>   	}
>   

[PATCH 3/7] udevng/gobi: log which iface is missing to succeed detection

[Alexander Couzens]

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

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

diff --git a/plugins/udevng.c b/plugins/udevng.c
index e8b800c1..4d928d02 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -224,8 +224,15 @@ static gboolean setup_gobi(struct modem_info *modem)
 		}
 	}
 
-	if (qmi == NULL || mdm == NULL || net == NULL)
+	if (qmi == NULL || mdm == NULL || net == NULL) {
+		if (!qmi)
+			DBG("mandatory qmi interface missing");
+		if (!mdm)
+			DBG("mandatory mdm interface missing");
+		if (!net)
+			DBG("mandatory net interface missing");
 		return FALSE;
+	}
 
 	DBG("qmi=%s net=%s mdm=%s gps=%s diag=%s", qmi, net, mdm, gps, diag);
 
-- 
2.12.2

Re: [PATCH 3/7] udevng/gobi: log which iface is missing to succeed detection

[Denis Kenzior]

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

Hi Alexander,

On 05/01/2017 03:52 PM, Alexander Couzens wrote:
> ---
>  plugins/udevng.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index e8b800c1..4d928d02 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -224,8 +224,15 @@ static gboolean setup_gobi(struct modem_info *modem)
>  		}
>  	}
>
> -	if (qmi == NULL || mdm == NULL || net == NULL)
> +	if (qmi == NULL || mdm == NULL || net == NULL) {
> +		if (!qmi)
> +			DBG("mandatory qmi interface missing");
> +		if (!mdm)
> +			DBG("mandatory mdm interface missing");
> +		if (!net)
> +			DBG("mandatory net interface missing");
>  		return FALSE;
> +	}

This is really not our style.  If this is really needed, lets just have 
separate if statements for each condition. e.g.

if (qmi == NULL) {
	DBG(..);
	return FALSE;
}

if (...) {
...
}

Also, please combine this with patch 4 to avoid some churn.  gobi 
doesn't use Diag or Modem.  So we might as well just take that out for now.

>
>  	DBG("qmi=%s net=%s mdm=%s gps=%s diag=%s", qmi, net, mdm, gps, diag);
>
>

Regards,
-Denis

[PATCH 4/7] udevng/gobi: make modem and diag interface optional

[Alexander Couzens]

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

Neither the modem nor the diag interface is used by the gobi driver.
---
 plugins/udevng.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 4d928d02..bd954f4b 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -224,11 +224,9 @@ static gboolean setup_gobi(struct modem_info *modem)
 		}
 	}
 
-	if (qmi == NULL || mdm == NULL || net == NULL) {
+	if (qmi == NULL || net == NULL) {
 		if (!qmi)
 			DBG("mandatory qmi interface missing");
-		if (!mdm)
-			DBG("mandatory mdm interface missing");
 		if (!net)
 			DBG("mandatory net interface missing");
 		return FALSE;
@@ -236,11 +234,21 @@ static gboolean setup_gobi(struct modem_info *modem)
 
 	DBG("qmi=%s net=%s mdm=%s gps=%s diag=%s", qmi, net, mdm, gps, diag);
 
+	/* mandatory interface */
 	ofono_modem_set_string(modem->modem, "Device", qmi);
-	ofono_modem_set_string(modem->modem, "Modem", mdm);
-	ofono_modem_set_string(modem->modem, "Diag", diag);
 	ofono_modem_set_string(modem->modem, "NetworkInterface", net);
 
+	/* optional */
+	if (diag)
+		ofono_modem_set_string(modem->modem, "Diag", diag);
+	else
+		DBG("optional diag interface missing");
+
+	if (mdm)
+		ofono_modem_set_string(modem->modem, "Modem", mdm);
+	else
+		DBG("optional mdm interface missing");
+
 	return TRUE;
 }
 
-- 
2.12.2

[PATCH 5/7] udevng/gobi: assign GPS device to the modem object

[Alexander Couzens]

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

---
 plugins/udevng.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index bd954f4b..9418bb5d 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -249,6 +249,11 @@ static gboolean setup_gobi(struct modem_info *modem)
 	else
 		DBG("optional mdm interface missing");
 
+	if (gps)
+		ofono_modem_set_string(modem->modem, "GPS", gps);
+	else
+		DBG("optional gps interface missing");
+
 	return TRUE;
 }
 
-- 
2.12.2

[PATCH 6/7] udevng/gobi: allow to detect ec20 tty devices

[Alexander Couzens]

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

The Quectel EC20 uses the same usb id as some gobi 2000 modules (05c6:9215).
---
 plugins/udevng.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 9418bb5d..9c106060 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -214,12 +214,20 @@ static gboolean setup_gobi(struct modem_info *modem)
 			net = info->devnode;
 		else if (g_strcmp0(info->subsystem, "tty") == 0) {
 			if (g_strcmp0(info->interface, "255/255/255") == 0) {
-				if (g_strcmp0(info->number, "01") == 0)
-					diag = info->devnode;
+				if (g_strcmp0(info->number, "00") == 0)
+					diag = info->devnode; /* ec20 */
+				else if (g_strcmp0(info->number, "01") == 0)
+					diag = info->devnode; /* gobi */
 				else if (g_strcmp0(info->number, "02") == 0)
-					mdm = info->devnode;
+					mdm = info->devnode; /* gobi */
 				else if (g_strcmp0(info->number, "03") == 0)
-					gps = info->devnode;
+					gps = info->devnode; /* gobi */
+			} else if (g_strcmp0(info->interface, "255/0/0") == 0) {
+				if (g_strcmp0(info->number, "01") == 0)
+					gps = info->devnode; /* ec20 */
+				if (g_strcmp0(info->number, "02") == 0)
+					mdm = info->devnode; /* ec20 */
+				/* ignore the 3rd device second AT/mdm iface */
 			}
 		}
 	}
-- 
2.12.2

[PATCH 7/7] plugins/gobi: add the qmi type to the debug output of discover_cb()

[Alexander Couzens]

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

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

diff --git a/plugins/gobi.c b/plugins/gobi.c
index df35f943..a4985990 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -262,7 +262,8 @@ static void discover_cb(uint8_t count, const struct qmi_version *list,
 	DBG("");
 
 	for (i = 0; i < count; i++) {
-		DBG("%s %d.%d", list[i].name, list[i].major, list[i].minor);
+		DBG("%s %d.%d - %d", list[i].name, list[i].major, list[i].minor,
+				list[i].type);
 
 		switch (list[i].type) {
 		case QMI_SERVICE_DMS:
-- 
2.12.2

Re: [PATCH 1/7] udevng/gobi: improve debug output

[Denis Kenzior]

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

Hi Alexander,

On 05/01/2017 03:52 PM, Alexander Couzens wrote:
> Add info->sysattr and info->subsystem to the log message.
> ---
>  plugins/udevng.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>

I went ahead and applied patches 1, 2, 6 & 7.

Regards,
-Denis