[PATCH 1/3] udevng: Add usbmisc scan for QMI devices

[PATCH 1/3] udevng: Add usbmisc scan for QMI devices

[Martin Chaplet]

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

cdc-wdm0 are now usbmisc devices
---
 plugins/udevng.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index e43d045..9643b0a 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1217,7 +1217,7 @@ static void check_device(struct udev_device *device)
 			return;
 	}
 
-	if (g_str_equal(bus, "usb") == TRUE)
+	if ((g_str_equal(bus, "usb") == TRUE) || (g_str_equal(bus, "usbmisc") == TRUE))
 		check_usb_device(device);
 }
 
@@ -1267,6 +1267,7 @@ static void enumerate_devices(struct udev *context)
 
 	udev_enumerate_add_match_subsystem(enumerate, "tty");
 	udev_enumerate_add_match_subsystem(enumerate, "usb");
+	udev_enumerate_add_match_subsystem(enumerate, "usbmisc");
 	udev_enumerate_add_match_subsystem(enumerate, "net");
 
 	udev_enumerate_scan_devices(enumerate);
@@ -1389,6 +1390,7 @@ static int detect_init(void)
 
 	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "tty", NULL);
 	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "usb", NULL);
+	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "usbmisc", NULL);
 	udev_monitor_filter_add_match_subsystem_devtype(udev_mon, "net", NULL);
 
 	udev_monitor_filter_update(udev_mon);
-- 
1.9.1

[PATCH 2/3] Gobi: Enable SIM driver "manual" config

[Martin Chaplet]

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

Let modem detect part overload default SIM driver by using an additional
property named "Simdriver".
It allows QMI modems with buggy UIM interface to be supported by forcing
legacy driver.
---
 plugins/gobi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index 4daa459..09c9d95 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -413,12 +413,16 @@ error:
 static void gobi_pre_sim(struct ofono_modem *modem)
 {
 	struct gobi_data *data = ofono_modem_get_data(modem);
+	const char * simdriver;
 
 	DBG("%p", modem);
 
 	ofono_devinfo_create(modem, 0, "qmimodem", data->device);
 
-	if (data->features & GOBI_UIM)
+	simdriver = ofono_modem_get_string(modem,"Simdriver");
+	if(simdriver != NULL)
+		ofono_sim_create(modem, 0, simdriver, data->device);
+	else if (data->features & GOBI_UIM)
 		ofono_sim_create(modem, 0, "qmimodem", data->device);
 	else if (data->features & GOBI_DMS)
 		ofono_sim_create(modem, 0, "qmimodem-legacy", data->device);
-- 
1.9.1

Re: [PATCH 2/3] Gobi: Enable SIM driver "manual" config

[Denis Kenzior]

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

Hi Martin,

On 05/02/2016 09:42 AM, Martin Chaplet wrote:
> Let modem detect part overload default SIM driver by using an additional
> property named "Simdriver".
> It allows QMI modems with buggy UIM interface to be supported by forcing
> legacy driver.
> ---
>   plugins/gobi.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/gobi.c b/plugins/gobi.c
> index 4daa459..09c9d95 100644
> --- a/plugins/gobi.c
> +++ b/plugins/gobi.c
> @@ -413,12 +413,16 @@ error:
>   static void gobi_pre_sim(struct ofono_modem *modem)
>   {
>   	struct gobi_data *data = ofono_modem_get_data(modem);
> +	const char * simdriver;
>
>   	DBG("%p", modem);
>
>   	ofono_devinfo_create(modem, 0, "qmimodem", data->device);
>
> -	if (data->features & GOBI_UIM)
> +	simdriver = ofono_modem_get_string(modem,"Simdriver");

Can we name this ForceSimLegacy and make it a boolean?

> +	if(simdriver != NULL)
> +		ofono_sim_create(modem, 0, simdriver, data->device);
> +	else if (data->features & GOBI_UIM)
>   		ofono_sim_create(modem, 0, "qmimodem", data->device);
>   	else if (data->features & GOBI_DMS)
>   		ofono_sim_create(modem, 0, "qmimodem-legacy", data->device);
>

Regards,
-Denis

Re: [PATCH 2/3] Gobi: Enable SIM driver "manual" config

[Martin Chaplet]

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

Hi Denis,

On 02/05/2016 17:13, Denis Kenzior wrote:
>
>>
>> diff --git a/plugins/gobi.c b/plugins/gobi.c
>> index 4daa459..09c9d95 100644
>> --- a/plugins/gobi.c
>> +++ b/plugins/gobi.c
>> @@ -413,12 +413,16 @@ error:
>>   static void gobi_pre_sim(struct ofono_modem *modem)
>>   {
>>       struct gobi_data *data = ofono_modem_get_data(modem);
>> +    const char * simdriver;
>>
>>       DBG("%p", modem);
>>
>>       ofono_devinfo_create(modem, 0, "qmimodem", data->device);
>>
>> -    if (data->features & GOBI_UIM)
>> +    simdriver = ofono_modem_get_string(modem,"Simdriver");
>
> Can we name this ForceSimLegacy and make it a boolean?
>

Yes of course. Actually, it was my first implementation ... and then, I
thought this type of "overload" can be interested for other modem drivers.
So I tried to make it generic through a string.
But I agree using a boolean is simplier and more reliable (avoid the use
of an unregistered driver).

>
> Regards,
> -Denis
>

Thanks,

Martin

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1904 bytes --]

[-- Attachment #3: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/3] Add support for Sierra MC73xx QMI modems

[Martin Chaplet]

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

Udevng layer is changed in order to support Sierra QMI modems like MC73cxx.
Identically to Huawei modems, these modems are parsed by setup_sierra.
If QMI interface is detected, the Gobi modem driver is selected.
In order to keep setup function as light as possible, real interface
matching is done by udev rule.

Unfortunately, MC73xx chips seem to have a broken QMI UIM interface.
The qmimodem-legacy is so forced in setup function.
---
 plugins/ofono.rules | 10 ++++++++++
 plugins/udevng.c    | 25 ++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/plugins/ofono.rules b/plugins/ofono.rules
index 3ed9f16..4c5e0e3 100644
--- a/plugins/ofono.rules
+++ b/plugins/ofono.rules
@@ -16,6 +16,16 @@ KERNEL=="shrm0", ENV{OFONO_DRIVER}="u8500"
 
 LABEL="ofono_isi_end"
 
+# Tag QMI devices
+SUBSYSTEM=="usbmisc", KERNEL=="cdc-wdm*", ENV{OFONO_LABEL}="qmi"
+
+# Sierra MC73xx (wwan0 is buggy, AT interface is ttyUSB2)
+ENV{ID_MODEL}!="MC73*", GOTO="sierra_end"
+SUBSYSTEM=="net", ENV{ID_USB_INTERFACE_NUM}=="0a", ENV{OFONO_LABEL}="net"
+SUBSYSTEM=="tty", ENV{ID_USB_INTERFACE_NUM}=="00", ENV{OFONO_LABEL}="diag"
+SUBSYSTEM=="tty", ENV{ID_USB_INTERFACE_NUM}=="03", ENV{OFONO_LABEL}="modem"
+LABEL="sierra_end"
+
 SUBSYSTEM!="usb", GOTO="ofono_end"
 ENV{DEVTYPE}!="usb_device", GOTO="ofono_end"
 
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 9643b0a..409ff22 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -219,7 +219,7 @@ static gboolean setup_gobi(struct modem_info *modem)
 
 static gboolean setup_sierra(struct modem_info *modem)
 {
-	const char *mdm = NULL, *app = NULL, *net = NULL, *diag = NULL;
+	const char *mdm = NULL, *app = NULL, *net = NULL, *diag = NULL, *qmi = NULL;
 	GSList *list;
 
 	DBG("%s", modem->syspath);
@@ -230,7 +230,15 @@ static gboolean setup_sierra(struct modem_info *modem)
 		DBG("%s %s %s %s", info->devnode, info->interface,
 						info->number, info->label);
 
-		if (g_strcmp0(info->interface, "255/255/255") == 0) {
+		if (g_strcmp0(info->label, "modem") == 0)
+			mdm = info->devnode;
+		else if (g_strcmp0(info->label, "net") == 0)
+			net = info->devnode;
+		else if (g_strcmp0(info->label, "diag") == 0)
+			diag = info->devnode;
+		else if (g_strcmp0(info->label, "qmi") == 0)
+			qmi = info->devnode;
+		else if (g_strcmp0(info->interface, "255/255/255") == 0) {
 			if (g_strcmp0(info->number, "01") == 0)
 				diag = info->devnode;
 			if (g_strcmp0(info->number, "03") == 0)
@@ -242,11 +250,20 @@ static gboolean setup_sierra(struct modem_info *modem)
 		}
 	}
 
+	if (qmi != NULL && net != NULL) {
+		ofono_modem_set_driver(modem->modem, "gobi");
+		/* Fixup SIM interface for Sierra QMI devices */
+		ofono_modem_set_string(modem->modem, "Simdriver", "qmimodem-legacy");
+		goto done;
+	}
+
 	if (mdm == NULL || net == NULL)
 		return FALSE;
 
-	DBG("modem=%s app=%s net=%s diag=%s", mdm, app, net, diag);
+done:
+	DBG("modem=%s app=%s net=%s diag=%s qmi=%s", mdm, app, net, diag, qmi);
 
+	ofono_modem_set_string(modem->modem, "Device", qmi);
 	ofono_modem_set_string(modem->modem, "Modem", mdm);
 	ofono_modem_set_string(modem->modem, "App", app);
 	ofono_modem_set_string(modem->modem, "Diag", diag);
@@ -1096,6 +1113,8 @@ static struct {
 	{ "hso",	"hso"				},
 	{ "gobi",	"qmi_wwan"			},
 	{ "gobi",	"qcserial"			},
+	{ "sierra",	"qmi_wwan",	"1199"		},
+	{ "sierra",	"qcserial",	"1199"		},
 	{ "sierra",	"sierra"			},
 	{ "sierra",	"sierra_net"			},
 	{ "option",	"option",	"0af0"		},
-- 
1.9.1

Re: [PATCH 3/3] Add support for Sierra MC73xx QMI modems

[Denis Kenzior]

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

Hi Martin,

On 05/02/2016 09:42 AM, Martin Chaplet wrote:
> Udevng layer is changed in order to support Sierra QMI modems like MC73cxx.
> Identically to Huawei modems, these modems are parsed by setup_sierra.
> If QMI interface is detected, the Gobi modem driver is selected.
> In order to keep setup function as light as possible, real interface
> matching is done by udev rule.

This is a bit unusual.  ofono.rules is now only used with the old 
plugins/udev.c detection logic.  udev.c is only really used for 
serial-based devices, not USB.

Can we add the logic directly to udevng.c and not depend on ofono.rules?

>
> Unfortunately, MC73xx chips seem to have a broken QMI UIM interface.
> The qmimodem-legacy is so forced in setup function.
> ---
>   plugins/ofono.rules | 10 ++++++++++
>   plugins/udevng.c    | 25 ++++++++++++++++++++++---
>   2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/plugins/ofono.rules b/plugins/ofono.rules
> index 3ed9f16..4c5e0e3 100644
> --- a/plugins/ofono.rules
> +++ b/plugins/ofono.rules
> @@ -16,6 +16,16 @@ KERNEL=="shrm0", ENV{OFONO_DRIVER}="u8500"
>
>   LABEL="ofono_isi_end"
>
> +# Tag QMI devices
> +SUBSYSTEM=="usbmisc", KERNEL=="cdc-wdm*", ENV{OFONO_LABEL}="qmi"
> +
> +# Sierra MC73xx (wwan0 is buggy, AT interface is ttyUSB2)
> +ENV{ID_MODEL}!="MC73*", GOTO="sierra_end"
> +SUBSYSTEM=="net", ENV{ID_USB_INTERFACE_NUM}=="0a", ENV{OFONO_LABEL}="net"
> +SUBSYSTEM=="tty", ENV{ID_USB_INTERFACE_NUM}=="00", ENV{OFONO_LABEL}="diag"
> +SUBSYSTEM=="tty", ENV{ID_USB_INTERFACE_NUM}=="03", ENV{OFONO_LABEL}="modem"
> +LABEL="sierra_end"
> +
>   SUBSYSTEM!="usb", GOTO="ofono_end"
>   ENV{DEVTYPE}!="usb_device", GOTO="ofono_end"
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 9643b0a..409ff22 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -219,7 +219,7 @@ static gboolean setup_gobi(struct modem_info *modem)
>
>   static gboolean setup_sierra(struct modem_info *modem)
>   {
> -	const char *mdm = NULL, *app = NULL, *net = NULL, *diag = NULL;
> +	const char *mdm = NULL, *app = NULL, *net = NULL, *diag = NULL, *qmi = NULL;
>   	GSList *list;
>
>   	DBG("%s", modem->syspath);
> @@ -230,7 +230,15 @@ static gboolean setup_sierra(struct modem_info *modem)
>   		DBG("%s %s %s %s", info->devnode, info->interface,
>   						info->number, info->label);
>
> -		if (g_strcmp0(info->interface, "255/255/255") == 0) {
> +		if (g_strcmp0(info->label, "modem") == 0)
> +			mdm = info->devnode;
> +		else if (g_strcmp0(info->label, "net") == 0)
> +			net = info->devnode;
> +		else if (g_strcmp0(info->label, "diag") == 0)
> +			diag = info->devnode;
> +		else if (g_strcmp0(info->label, "qmi") == 0)
> +			qmi = info->devnode;
> +		else if (g_strcmp0(info->interface, "255/255/255") == 0) {
>   			if (g_strcmp0(info->number, "01") == 0)
>   				diag = info->devnode;
>   			if (g_strcmp0(info->number, "03") == 0)
> @@ -242,11 +250,20 @@ static gboolean setup_sierra(struct modem_info *modem)
>   		}
>   	}
>
> +	if (qmi != NULL && net != NULL) {
> +		ofono_modem_set_driver(modem->modem, "gobi");
> +		/* Fixup SIM interface for Sierra QMI devices */
> +		ofono_modem_set_string(modem->modem, "Simdriver", "qmimodem-legacy");

Lets do something like:

ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);

> +		goto done;
> +	}
> +
>   	if (mdm == NULL || net == NULL)
>   		return FALSE;
>
> -	DBG("modem=%s app=%s net=%s diag=%s", mdm, app, net, diag);
> +done:
> +	DBG("modem=%s app=%s net=%s diag=%s qmi=%s", mdm, app, net, diag, qmi);
>
> +	ofono_modem_set_string(modem->modem, "Device", qmi);
>   	ofono_modem_set_string(modem->modem, "Modem", mdm);
>   	ofono_modem_set_string(modem->modem, "App", app);
>   	ofono_modem_set_string(modem->modem, "Diag", diag);
> @@ -1096,6 +1113,8 @@ static struct {
>   	{ "hso",	"hso"				},
>   	{ "gobi",	"qmi_wwan"			},
>   	{ "gobi",	"qcserial"			},
> +	{ "sierra",	"qmi_wwan",	"1199"		},
> +	{ "sierra",	"qcserial",	"1199"		},
>   	{ "sierra",	"sierra"			},
>   	{ "sierra",	"sierra_net"			},
>   	{ "option",	"option",	"0af0"		},
>

Regards,
-Denis

Re: [PATCH 3/3] Add support for Sierra MC73xx QMI modems

[Martin Chaplet]

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

Hi Denis,

On 02/05/2016 17:20, Denis Kenzior wrote:
> Hi Martin,
>
> On 05/02/2016 09:42 AM, Martin Chaplet wrote:
>> Udevng layer is changed in order to support Sierra QMI modems like
>> MC73cxx.
>> Identically to Huawei modems, these modems are parsed by setup_sierra.
>> If QMI interface is detected, the Gobi modem driver is selected.
>> In order to keep setup function as light as possible, real interface
>> matching is done by udev rule.
>
> This is a bit unusual.  ofono.rules is now only used with the old
> plugins/udev.c detection logic.  udev.c is only really used for
> serial-based devices, not USB.
>
> Can we add the logic directly to udevng.c and not depend on ofono.rules?


Ok, I will probably need 'subsystem' udev property in this case.
I will re-send splitted patches.
>>
>> +    if (qmi != NULL && net != NULL) {
>> +        ofono_modem_set_driver(modem->modem, "gobi");
>> +        /* Fixup SIM interface for Sierra QMI devices */
>> +        ofono_modem_set_string(modem->modem, "Simdriver",
>> "qmimodem-legacy");
>
> Lets do something like:
>
> ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);

Ok

>
> Regards,
> -Denis
>

Thanks,

Martin


[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 2300 bytes --]

[-- Attachment #3: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/3] Gobi: Enable SIM driver forcing

[Martin Chaplet]

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

Let modem detect part overload default SIM driver by using an additional
property named "ForceSimLegacy" (boolean).

Signed-off-by: Martin Chaplet <m.chaplet@kerlink.fr>
---
 plugins/gobi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index 4daa459..aad423d 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -418,7 +418,7 @@ static void gobi_pre_sim(struct ofono_modem *modem)
 
 	ofono_devinfo_create(modem, 0, "qmimodem", data->device);
 
-	if (data->features & GOBI_UIM)
+	if ( (data->features & GOBI_UIM) && !ofono_modem_get_boolean(modem, "ForceSimLegacy") )
 		ofono_sim_create(modem, 0, "qmimodem", data->device);
 	else if (data->features & GOBI_DMS)
 		ofono_sim_create(modem, 0, "qmimodem-legacy", data->device);
-- 
1.9.1

[PATCH 2/3] Udevng: Improve modem properties detection

[Martin Chaplet]

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

 * Add driver's subsystem information (net, usb, tty, ...)
 * Improve interface number extraction by scanning also device and
 parent attributes

Signed-off-by: Martin Chaplet <m.chaplet@kerlink.fr>
---
 plugins/udevng.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 9643b0a..8ca4471 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -55,6 +55,7 @@ struct device_info {
 	char *number;
 	char *label;
 	char *sysattr;
+	char *subsystem;
 };
 
 static gboolean setup_isi(struct modem_info *modem)
@@ -939,6 +940,7 @@ static void destroy_modem(gpointer data)
 		g_free(info->number);
 		g_free(info->label);
 		g_free(info->sysattr);
+		g_free(info->subsystem);
 		g_free(info);
 
 		list->data = NULL;
@@ -997,9 +999,10 @@ static void add_device(const char *syspath, const char *devname,
 			const char *model, struct udev_device *device)
 {
 	struct udev_device *intf;
-	const char *devpath, *devnode, *interface, *number, *label, *sysattr;
+	const char *devpath, *devnode, *interface, *number, *label, *sysattr, *subsystem;
 	struct modem_info *modem;
 	struct device_info *info;
+	struct udev_device *parent;
 
 	devpath = udev_device_get_syspath(device);
 	if (devpath == NULL)
@@ -1037,7 +1040,19 @@ static void add_device(const char *syspath, const char *devname,
 	interface = udev_device_get_property_value(intf, "INTERFACE");
 	number = udev_device_get_property_value(device, "ID_USB_INTERFACE_NUM");
 
+	/* If environment variable is not set, get value from attributes (or parent's ones) */
+	if(number == NULL)
+	{
+		number = udev_device_get_sysattr_value(device, "bInterfaceNumber");
+		if(number == NULL)
+		{
+			parent = udev_device_get_parent(device);
+			number = udev_device_get_sysattr_value(parent, "bInterfaceNumber");
+		}
+	}
+
 	label = udev_device_get_property_value(device, "OFONO_LABEL");
+	subsystem = udev_device_get_subsystem(device);
 
 	if (modem->sysattr != NULL)
 		sysattr = udev_device_get_sysattr_value(device, modem->sysattr);
@@ -1059,6 +1074,7 @@ static void add_device(const char *syspath, const char *devname,
 	info->number = g_strdup(number);
 	info->label = g_strdup(label);
 	info->sysattr = g_strdup(sysattr);
+	info->subsystem = g_strdup(subsystem);
 
 	modem->devices = g_slist_insert_sorted(modem->devices, info,
 							compare_device);
-- 
1.9.1

Re: [PATCH 2/3] Udevng: Improve modem properties detection

[Denis Kenzior]

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

Hi Martin,

On 05/02/2016 11:33 AM, Martin Chaplet wrote:
>   * Add driver's subsystem information (net, usb, tty, ...)
>   * Improve interface number extraction by scanning also device and
>   parent attributes
>
> Signed-off-by: Martin Chaplet <m.chaplet@kerlink.fr>

No signed-off-by please.  See HACKING, 'Submitting Patches' section.

> ---
>   plugins/udevng.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 9643b0a..8ca4471 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -55,6 +55,7 @@ struct device_info {
>   	char *number;
>   	char *label;
>   	char *sysattr;
> +	char *subsystem;
>   };
>
>   static gboolean setup_isi(struct modem_info *modem)
> @@ -939,6 +940,7 @@ static void destroy_modem(gpointer data)
>   		g_free(info->number);
>   		g_free(info->label);
>   		g_free(info->sysattr);
> +		g_free(info->subsystem);
>   		g_free(info);
>
>   		list->data = NULL;
> @@ -997,9 +999,10 @@ static void add_device(const char *syspath, const char *devname,
>   			const char *model, struct udev_device *device)
>   {
>   	struct udev_device *intf;
> -	const char *devpath, *devnode, *interface, *number, *label, *sysattr;
> +	const char *devpath, *devnode, *interface, *number, *label, *sysattr, *subsystem;
>   	struct modem_info *modem;
>   	struct device_info *info;
> +	struct udev_device *parent;
>
>   	devpath = udev_device_get_syspath(device);
>   	if (devpath == NULL)
> @@ -1037,7 +1040,19 @@ static void add_device(const char *syspath, const char *devname,
>   	interface = udev_device_get_property_value(intf, "INTERFACE");
>   	number = udev_device_get_property_value(device, "ID_USB_INTERFACE_NUM");
>
> +	/* If environment variable is not set, get value from attributes (or parent's ones) */
> +	if(number == NULL)
> +	{

Not our style.  oFono uses the Linux Kernel coding style, so the '{' 
should be on the same line as the if.

> +		number = udev_device_get_sysattr_value(device, "bInterfaceNumber");
> +		if(number == NULL)
> +		{

as above

> +			parent = udev_device_get_parent(device);
> +			number = udev_device_get_sysattr_value(parent, "bInterfaceNumber");
> +		}
> +	}
> +
>   	label = udev_device_get_property_value(device, "OFONO_LABEL");
> +	subsystem = udev_device_get_subsystem(device);
>
>   	if (modem->sysattr != NULL)
>   		sysattr = udev_device_get_sysattr_value(device, modem->sysattr);
> @@ -1059,6 +1074,7 @@ static void add_device(const char *syspath, const char *devname,
>   	info->number = g_strdup(number);
>   	info->label = g_strdup(label);
>   	info->sysattr = g_strdup(sysattr);
> +	info->subsystem = g_strdup(subsystem);
>
>   	modem->devices = g_slist_insert_sorted(modem->devices, info,
>   							compare_device);
>

Regards,
-Denis

Re: [PATCH 2/3] Udevng: Improve modem properties detection

[Martin Chaplet]

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

Hi Denis,

On 03/05/2016 05:23, Denis Kenzior wrote:
> Hi Martin,
>
> On 05/02/2016 11:33 AM, Martin Chaplet wrote:
>>   * Add driver's subsystem information (net, usb, tty, ...)
>>   * Improve interface number extraction by scanning also device and
>>   parent attributes
>>
>> Signed-off-by: Martin Chaplet <m.chaplet@kerlink.fr>
>
> No signed-off-by please.  See HACKING, 'Submitting Patches' section.
>
>>
>>
>>       devpath = udev_device_get_syspath(device);
>>       if (devpath == NULL)
>> @@ -1037,7 +1040,19 @@ static void add_device(const char *syspath,
>> const char *devname,
>>       interface = udev_device_get_property_value(intf, "INTERFACE");
>>       number = udev_device_get_property_value(device,
>> "ID_USB_INTERFACE_NUM");
>>
>> +    /* If environment variable is not set, get value from attributes
>> (or parent's ones) */
>> +    if(number == NULL)
>> +    {
>
> Not our style.  oFono uses the Linux Kernel coding style, so the '{'
> should be on the same line as the if.
>
>> +        number = udev_device_get_sysattr_value(device,
>> "bInterfaceNumber");
>> +        if(number == NULL)
>> +        {
>
> as above
>

OK, sorry for those mistakes and thanks for your review.
I will re-send cleaner patches.

Regards,

Martin

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 2337 bytes --]

[-- Attachment #3: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/3] Add support for Sierra MC73xx QMI modems

[Martin Chaplet]

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

Udevng layer is changed in order to support Sierra QMI modems like
MC73cxx.
Identically to Huawei modems, these modems are parsed by setup_sierra.
If QMI interface is detected, the Gobi modem driver is selected.

Unfortunately, MC73xx chips seem to have a broken QMI UIM interface.
The qmimodem-legacy is so forced in setup function.

Signed-off-by: Martin Chaplet <m.chaplet@kerlink.fr>
---
 plugins/udevng.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 8ca4471..d95ac98 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -220,7 +220,7 @@ static gboolean setup_gobi(struct modem_info *modem)
 
 static gboolean setup_sierra(struct modem_info *modem)
 {
-	const char *mdm = NULL, *app = NULL, *net = NULL, *diag = NULL;
+	const char *mdm = NULL, *app = NULL, *net = NULL, *diag = NULL, *qmi = NULL;
 	GSList *list;
 
 	DBG("%s", modem->syspath);
@@ -228,8 +228,8 @@ static gboolean setup_sierra(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", info->devnode, info->interface,
+						info->number, info->label, info->subsystem);
 
 		if (g_strcmp0(info->interface, "255/255/255") == 0) {
 			if (g_strcmp0(info->number, "01") == 0)
@@ -240,14 +240,30 @@ static gboolean setup_sierra(struct modem_info *modem)
 				app = info->devnode;
 			else if (g_strcmp0(info->number, "07") == 0)
 				net = info->devnode;
+			else if (g_strcmp0(info->number, "0a") == 0)
+			{
+				if (g_strcmp0(info->subsystem, "net") == 0)
+					net = info->devnode;
+				else if (g_strcmp0(info->subsystem, "usbmisc") == 0)
+					qmi = info->devnode;
+			}
 		}
 	}
 
+	if (qmi != NULL && net != NULL) {
+		ofono_modem_set_driver(modem->modem, "gobi");
+		/* Fixup SIM interface for Sierra QMI devices */
+		ofono_modem_set_boolean(modem->modem, "ForceSimLegacy", TRUE);
+		goto done;
+	}
+
 	if (mdm == NULL || net == NULL)
 		return FALSE;
 
-	DBG("modem=%s app=%s net=%s diag=%s", mdm, app, net, diag);
+done:
+	DBG("modem=%s app=%s net=%s diag=%s qmi=%s", mdm, app, net, diag, qmi);
 
+	ofono_modem_set_string(modem->modem, "Device", qmi);
 	ofono_modem_set_string(modem->modem, "Modem", mdm);
 	ofono_modem_set_string(modem->modem, "App", app);
 	ofono_modem_set_string(modem->modem, "Diag", diag);
@@ -1112,6 +1128,8 @@ static struct {
 	{ "hso",	"hso"				},
 	{ "gobi",	"qmi_wwan"			},
 	{ "gobi",	"qcserial"			},
+	{ "sierra",	"qmi_wwan",	"1199"		},
+	{ "sierra",	"qcserial",	"1199"		},
 	{ "sierra",	"sierra"			},
 	{ "sierra",	"sierra_net"			},
 	{ "option",	"option",	"0af0"		},
-- 
1.9.1

Re: [PATCH 1/3] Gobi: Enable SIM driver forcing

[Denis Kenzior]

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

Hi Martin,

On 05/02/2016 11:33 AM, Martin Chaplet wrote:
> Let modem detect part overload default SIM driver by using an additional
> property named "ForceSimLegacy" (boolean).
>
> Signed-off-by: Martin Chaplet <m.chaplet@kerlink.fr>

No Signed-off-by please.

> ---
>   plugins/gobi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/plugins/gobi.c b/plugins/gobi.c
> index 4daa459..aad423d 100644
> --- a/plugins/gobi.c
> +++ b/plugins/gobi.c
> @@ -418,7 +418,7 @@ static void gobi_pre_sim(struct ofono_modem *modem)
>
>   	ofono_devinfo_create(modem, 0, "qmimodem", data->device);
>
> -	if (data->features & GOBI_UIM)
> +	if ( (data->features & GOBI_UIM) && !ofono_modem_get_boolean(modem, "ForceSimLegacy") )

Not our style.  Also the logic would be cleaner if we do something like:

const char *sim_driver;

if (data->features & GOBI_UIM)
	sim_driver = "qmimodem";
else if (data->features & GOBI_DMS)
	sim_driver = "qmimodem-legacy";

if (ofono_modem_get_boolean(modem, "ForceSimLegacy"))
	sim_driver = "qmimodem-legacy";

ofono_sim_create(...);

>   		ofono_sim_create(modem, 0, "qmimodem", data->device);
>   	else if (data->features & GOBI_DMS)
>   		ofono_sim_create(modem, 0, "qmimodem-legacy", data->device);
>

Regards,
-Denis

Re: [PATCH 3/3] Add support for Sierra MC73xx QMI modems

[=?unknown-8bit?q?Bj=C3=B8rn?= Mork]

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

Martin Chaplet <m.chaplet@kerlink.fr> writes:

> +# Sierra MC73xx (wwan0 is buggy,

I suspect this is a raw-ip vs 802.3 issue. For example: wwan0 might be
configured for raw-ip if firmware autoconnect is enabled. You should be
able to detect this with a QMI_WDA "Get Data Format" request.

In general, both interfaces 8 and 10 are usable on these modems.



Bjørn

Re: [PATCH 3/3] Add support for Sierra MC73xx QMI modems

[Martin Chaplet]

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

Hi Bjørn,

Firstly, thank you for your previous patches which help me a lot.

On 02/05/2016 22:24, Bjørn Mork wrote:
> Martin Chaplet <m.chaplet@kerlink.fr> writes:
>
>> +# Sierra MC73xx (wwan0 is buggy,
> I suspect this is a raw-ip vs 802.3 issue. For example: wwan0 might be
> configured for raw-ip if firmware autoconnect is enabled. You should be
> able to detect this with a QMI_WDA "Get Data Format" request.
>
> In general, both interfaces 8 and 10 are usable on these modems.

You are so right !

However, WDA service is not yet supported in qmimodem.
So I think we can keep the interface matching on QMI interface no 10 for
now because  using interface 8 will imply changing its default link
layer at each boot.

What's your opinion ?

Best regards,

Martin

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1384 bytes --]

[-- Attachment #3: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] Add support for Sierra MC73xx QMI modems

[=?unknown-8bit?q?Bj=C3=B8rn?= Mork]

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

Martin Chaplet <m.chaplet@kerlink.fr> writes:

> Firstly, thank you for your previous patches which help me a lot.

And thank you for resurrecting it!

> On 02/05/2016 22:24, Bjørn Mork wrote:
>> Martin Chaplet <m.chaplet@kerlink.fr> writes:
>>
>>> +# Sierra MC73xx (wwan0 is buggy,
>> I suspect this is a raw-ip vs 802.3 issue. For example: wwan0 might be
>> configured for raw-ip if firmware autoconnect is enabled. You should be
>> able to detect this with a QMI_WDA "Get Data Format" request.
>>
>> In general, both interfaces 8 and 10 are usable on these modems.
>
> You are so right !
>
> However, WDA service is not yet supported in qmimodem.
> So I think we can keep the interface matching on QMI interface no 10 for
> now because  using interface 8 will imply changing its default link
> layer at each boot.
>
> What's your opinion ?

I believe there is no way around verifying the data format if you want
this to be robust. The 'data format' setting does not depend on modem
name or id or whatever.  It is configurable, and defaulting to the last
used value stored in NVRAM.  This setting will therefore vary among
identical modems, unless you force it yourself.

Also note that there is no 802.3 support at all in the newer MC74xx
series.  A 'raw-ip' mode was therefore added to the qmi_wwan driver in
Linux v4.5.  It changes the interface type though, so userspace
applications need to be aware.  Which is also one reason this driver
mode is never applied automatically - userspace must request it.

IMHO, it would be nice if the oFono support was flexible enough to
support all three generations Sierra Wireless QMI modems: MC77xx, MC73xx
and MC74xx.  But that's probably more suitable for a long term goal.
Initially concentrating on the MC73xx support makes sense.

FWIW, your approach looks fine to me.  But I'm definitely no oFono
expert :)


Bjørn

Re: [PATCH 1/3] udevng: Add usbmisc scan for QMI devices

[Denis Kenzior]

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

Hi Martin,

On 05/02/2016 09:42 AM, Martin Chaplet wrote:
> cdc-wdm0 are now usbmisc devices
> ---
>   plugins/udevng.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>

Applied, thanks.

Regards,
-Denis