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

[Denis Kenzior <denkenz@gmail.com>]

[-- 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