Re: [MM qmi-support] "Set Operating Mode" on shutdown and more..

[Bjørn Mork <bjorn mork no>]

Aleksander Morgado <aleksander lanedo com> writes:

> Hey!
>
> You're truly trying *a lot* of untested code :-)

Oh, yeah.  It's a lot easier to find bugs then :-)

> The new API allows users to create more than one bearer; when it's done
> we'll allow to do all that. I must say, I didn't really cover yet the
> case of multiple QMI ports in the same device, that's why you're only
> seeing one there. Another thing for my TODO list.


Thanks.


>> Do also note that the above setup is working by pure luck.  The code
>> really should try to match the wwanX and cdc-wdmY devices by USB
>> interface and not by USB device.  If we ignore the unfortunate 3.4 and
>> 3.5 kernels, then a matching wwanX and cdc-wdmY set will always share
>> the same parent USB interface on QMI devices.
>> 
>> Having the same parent USB device is *not* sufficient.  You cannot
>> control wwan0 using cdc-wdm1 in the above example.
>> 
>
>
> Oh, interesting. So, each cdc-wdm *always* has a specific wwan port
> paired, then, right? 

Yes, for QMI devices.  Keep in mind that cdc-wdm devices aren't
necessarily QMI.  We already have the AT command type from Ericsson.
And I hope we soon will have MBIM cdc-wdm devices too.

Unfortunately, there are exceptions to the USB interface grouping in
Linux 3.4 and 3.5 due to the split between the qmi_wwan and cdc_wdm
drivers for devices with separate control and data interface.  But AFAIK
there is no multi-port device with separate control and data interface.

> I'll give a look at the logic of multiple QMI/wwan
> ports one of these days.

Yes, I hope there will be more of them in the future.   But I believe
many current devices can support simultaneous ppp and wwan, and as such
be seen as having multiple data ports.

> Do you (does anyonoe) know any individual/company who would be willing
> to sponsor some of these newer QMI-powered modems for developers?

Nope, sorry.  MBIM devices would also be nice, but I don't even know
where to buy one of those.

>> 3) I still wonder how to handle the usb => usbmisc transition.  For now
>> I've just been searching and replacing, but that is of course not
>> supportable.  And it isn't really a one-to-one replacement either.
>> There are places where "usb" actually refer to the subsystem and not the
>> class.
>> 
>> Should "usbmisc" be added as an additional device class, keeping "usb",
>> or should there be some kernel version checking code there?
>> 
>
> Can you give me your diff to look at where you did the changes? I guess
> we should try to support both cases, regardless of the kernel version,
> if somehow possible.


This is my diff which is sort of working for me, but I haven't sanity
checked it so there are probably errors here:


diff --git a/plugins/generic/mm-plugin-generic.c b/plugins/generic/mm-plugin-generic.c
index e4d4716..ef44cae 100644
--- a/plugins/generic/mm-plugin-generic.c
+++ b/plugins/generic/mm-plugin-generic.c
@@ -70,7 +70,7 @@ create_modem (MMPlugin *self,
 G_MODULE_EXPORT MMPlugin *
 mm_plugin_create (void)
 {
-    static const gchar *subsystems[] = { "tty", "net", "usb", NULL };
+    static const gchar *subsystems[] = { "tty", "net", "usbmisc", NULL };
 
     return MM_PLUGIN (
         g_object_new (MM_TYPE_PLUGIN_GENERIC,
diff --git a/plugins/gobi/mm-plugin-gobi.c b/plugins/gobi/mm-plugin-gobi.c
index e8fa108..cab0bfb 100644
--- a/plugins/gobi/mm-plugin-gobi.c
+++ b/plugins/gobi/mm-plugin-gobi.c
@@ -61,7 +61,7 @@ create_modem (MMPlugin *self,
 G_MODULE_EXPORT MMPlugin *
 mm_plugin_create (void)
 {
-    static const gchar *subsystems[] = { "tty", "net", "usb", NULL };
+    static const gchar *subsystems[] = { "tty", "net", "usbmisc", NULL };
     static const gchar *drivers[] = { "qcserial", NULL };
 
     return MM_PLUGIN (
diff --git a/plugins/pantech/mm-plugin-pantech.c b/plugins/pantech/mm-plugin-pantech.c
index b7957f0..3f37e84 100644
--- a/plugins/pantech/mm-plugin-pantech.c
+++ b/plugins/pantech/mm-plugin-pantech.c
@@ -85,7 +85,7 @@ grab_port (MMPlugin *self,
 G_MODULE_EXPORT MMPlugin *
 mm_plugin_create (void)
 {
-    static const gchar *subsystems[] = { "tty", "net", "usb", NULL };
+    static const gchar *subsystems[] = { "tty", "net", "usbmisc", NULL };
     static const guint16 vendor_ids[] = { 0x106c, 0 };
 
     return MM_PLUGIN (
diff --git a/src/80-mm-candidate.rules b/src/80-mm-candidate.rules
index 01ba912..58287eb 100644
--- a/src/80-mm-candidate.rules
+++ b/src/80-mm-candidate.rules
@@ -11,6 +11,6 @@ ACTION!="add|change|move", GOTO="mm_candidate_end"
 
 SUBSYSTEM=="tty", ENV{ID_MM_CANDIDATE}="1"
 SUBSYSTEM=="net", ENV{ID_MM_CANDIDATE}="1"
-KERNEL=="cdc-wdm*", SUBSYSTEM=="usb", ENV{ID_MM_CANDIDATE}="1"
+KERNEL=="cdc-wdm*", SUBSYSTEM=="usbmisc", ENV{ID_MM_CANDIDATE}="1"
 
 LABEL="mm_candidate_end"
diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c
index 70cdb0e..adab407 100644
--- a/src/mm-base-modem.c
+++ b/src/mm-base-modem.c
@@ -165,7 +165,7 @@ mm_base_modem_grab_port (MMBaseModem *self,
     /* Only allow 'tty' and 'net' ports */
     if (!g_str_equal (subsys, "net") &&
         !g_str_equal (subsys, "tty") &&
-        !(g_str_equal (subsys, "usb") && g_str_has_prefix (name, "cdc-wdm"))) {
+        !(g_str_equal (subsys, "usbmisc") && g_str_has_prefix (name, "cdc-wdm"))) {
         g_set_error (error,
                      MM_CORE_ERROR,
                      MM_CORE_ERROR_UNSUPPORTED,
@@ -234,7 +234,7 @@ mm_base_modem_grab_port (MMBaseModem *self,
                                       NULL));
     }
     /* QMI ports... */
-    else if (g_str_equal (subsys, "usb") &&
+    else if (g_str_equal (subsys, "usbmisc") &&
              g_str_has_prefix (name, "cdc-wdm")) {
         port = MM_PORT (mm_qmi_port_new (name));
     } else
diff --git a/src/mm-device.c b/src/mm-device.c
index c3f6aaa..01cffbd 100644
--- a/src/mm-device.c
+++ b/src/mm-device.c
@@ -140,7 +140,7 @@ get_device_ids (GUdevDevice *device,
                 /* Platform devices don't usually have a VID/PID */
                 success = TRUE;
                 goto out;
-            } else if (!strcmp (parent_subsys, "usb") &&
+            } else if (!strcmp (parent_subsys, "usbmisc") &&
                        !strcmp (g_udev_device_get_driver (parent), "qmi_wwan")) {
                 /* Need to look for vendor/product in the parent of the QMI device */
                 GUdevDevice *qmi_parent;
diff --git a/src/mm-manager.c b/src/mm-manager.c
index 23ef8e6..9ef7179 100644
--- a/src/mm-manager.c
+++ b/src/mm-manager.c
@@ -329,7 +329,7 @@ device_removed (MMManager *self,
     subsys = g_udev_device_get_subsystem (udev_device);
     name = g_udev_device_get_name (udev_device);
 
-    if (!g_str_equal (subsys, "usb") ||
+    if (!g_str_equal (subsys, "usbmisc") ||
         (name && g_str_has_prefix (name, "cdc-wdm"))) {
         /* Handle tty/net/wdm port removal */
         device = find_device_by_port (self, udev_device);
@@ -386,14 +386,14 @@ handle_uevent (GUdevClient *client,
     /* A bit paranoid */
     subsys = g_udev_device_get_subsystem (device);
     g_return_if_fail (subsys != NULL);
-    g_return_if_fail (g_str_equal (subsys, "tty") || g_str_equal (subsys, "net") || g_str_equal (subsys, "usb"));
+    g_return_if_fail (g_str_equal (subsys, "tty") || g_str_equal (subsys, "net") || g_str_equal (subsys, "usbmisc"));
 
     /* We only care about tty/net and usb/cdc-wdm devices when adding modem ports,
      * but for remove, also handle usb parent device remove events
      */
     name = g_udev_device_get_name (device);
     if (   (g_str_equal (action, "add") || g_str_equal (action, "move") || g_str_equal (action, "change"))
-        && (!g_str_equal (subsys, "usb") || (name && g_str_has_prefix (name, "cdc-wdm"))))
+        && (!g_str_equal (subsys, "usbmisc") || (name && g_str_has_prefix (name, "cdc-wdm"))))
         device_added (self, device);
     else if (g_str_equal (action, "remove"))
         device_removed (self, device);
@@ -423,7 +423,7 @@ mm_manager_start (MMManager *manager)
     }
     g_list_free (devices);
 
-    devices = g_udev_client_query_by_subsystem (manager->priv->udev, "usb");
+    devices = g_udev_client_query_by_subsystem (manager->priv->udev, "usbmisc");
     for (iter = devices; iter; iter = g_list_next (iter)) {
         const gchar *name;
 
@@ -672,7 +672,7 @@ static void
 mm_manager_init (MMManager *manager)
 {
     MMManagerPrivate *priv;
-    const char *subsys[4] = { "tty", "net", "usb", NULL };
+    const char *subsys[4] = { "tty", "net", "usbmisc", NULL };
 
     /* Setup private data */
     manager->priv = priv = G_TYPE_INSTANCE_GET_PRIVATE ((manager),
diff --git a/src/mm-port-probe.c b/src/mm-port-probe.c
index 61ee3cb..cea60ca 100644
--- a/src/mm-port-probe.c
+++ b/src/mm-port-probe.c
@@ -1118,7 +1118,7 @@ mm_port_probe_is_at (MMPortProbe *self)
     subsys = g_udev_device_get_subsystem (self->priv->port);
     name = g_udev_device_get_name (self->priv->port);
     if (g_str_equal (subsys, "net") ||
-        (g_str_equal (subsys, "usb") &&
+        (g_str_equal (subsys, "usbmisc") &&
          g_str_has_prefix (name, "cdc-wdm")))
         return FALSE;
 
@@ -1153,7 +1153,7 @@ mm_port_probe_is_qcdm (MMPortProbe *self)
     subsys = g_udev_device_get_subsystem (self->priv->port);
     name = g_udev_device_get_name (self->priv->port);
     if (g_str_equal (subsys, "net") ||
-        (g_str_equal (subsys, "usb") &&
+        (g_str_equal (subsys, "usbmisc") &&
          g_str_has_prefix (name, "cdc-wdm")))
         return FALSE;
 
@@ -1172,7 +1172,7 @@ mm_port_probe_is_qmi (MMPortProbe *self)
 
     subsys = g_udev_device_get_subsystem (self->priv->port);
     name = g_udev_device_get_name (self->priv->port);
-    if (!g_str_equal (subsys, "usb") ||
+    if (!g_str_equal (subsys, "usbmisc") ||
         !name ||
         !g_str_has_prefix (name, "cdc-wdm"))
         return FALSE;
@@ -1207,7 +1207,7 @@ mm_port_probe_get_port_type (MMPortProbe *self)
     if (g_str_equal (subsys, "net"))
         return MM_PORT_TYPE_NET;
 
-    if (g_str_equal (subsys, "usb") &&
+    if (g_str_equal (subsys, "usbmisc") &&
         g_str_has_prefix (name, "cdc-wdm") &&
         self->priv->is_qmi)
         return MM_PORT_TYPE_QMI;
@@ -1266,7 +1266,7 @@ mm_port_probe_get_vendor (MMPortProbe *self)
     subsys = g_udev_device_get_subsystem (self->priv->port);
     name = g_udev_device_get_name (self->priv->port);
     if (g_str_equal (subsys, "net") ||
-        (g_str_equal (subsys, "usb") &&
+        (g_str_equal (subsys, "usbmisc") &&
          g_str_has_prefix (name, "cdc-wdm")))
         return NULL;
 
@@ -1286,7 +1286,7 @@ mm_port_probe_get_product (MMPortProbe *self)
     subsys = g_udev_device_get_subsystem (self->priv->port);
     name = g_udev_device_get_name (self->priv->port);
     if (g_str_equal (subsys, "net") ||
-        (g_str_equal (subsys, "usb") &&
+        (g_str_equal (subsys, "usbmisc") &&
          g_str_has_prefix (name, "cdc-wdm")))
         return NULL;
 
diff --git a/src/mm-port.h b/src/mm-port.h
index 182dd23..9325428 100644
--- a/src/mm-port.h
+++ b/src/mm-port.h
@@ -23,9 +23,9 @@ typedef enum { /*< underscore_name=mm_port_subsys >*/
     MM_PORT_SUBSYS_UNKNOWN = 0x0,
     MM_PORT_SUBSYS_TTY,
     MM_PORT_SUBSYS_NET,
-    MM_PORT_SUBSYS_USB,
+    MM_PORT_SUBSYS_USBMISC,
 
-    MM_PORT_SUBSYS_LAST = MM_PORT_SUBSYS_USB /*< skip >*/
+    MM_PORT_SUBSYS_LAST = MM_PORT_SUBSYS_USBMISC /*< skip >*/
 } MMPortSubsys;
 
 typedef enum { /*< underscore_name=mm_port_type >*/
diff --git a/src/mm-qmi-port.c b/src/mm-qmi-port.c
index 1465b8e..8d75ba2 100644
--- a/src/mm-qmi-port.c
+++ b/src/mm-qmi-port.c
@@ -323,7 +323,7 @@ mm_qmi_port_new (const gchar *name)
 {
     return MM_QMI_PORT (g_object_new (MM_TYPE_QMI_PORT,
                                       MM_PORT_DEVICE, name,
-                                      MM_PORT_SUBSYS, MM_PORT_SUBSYS_USB,
+                                      MM_PORT_SUBSYS, MM_PORT_SUBSYS_USBMISC,
                                       MM_PORT_TYPE, MM_PORT_TYPE_QMI,
                                       NULL));
 }