[MM] Boost Option/HSO probings

[Aleksander Morgado <aleksander lanedo com>]

Hey Dan,

I prepared a patch which would make the option/hso modem probing almost
instant, by moving the port type hint logic (the one reading the
per-port 'hsotype' file) to a custom init operation done for each tty
port when the probing sequence starts, instead of doing it once probing
is finished. With this patch we really rely on the information given by
the files; so e.g. if the 'hsotype' file for a given port tells us it's
"Control" we do assume the port is the primary AT port (we won't do any
AT probing in the port). Of course, if no 'hsotype' file is found for a
given port, we just launch the normal port probing sequence.

Another approach would be to grab the port type hint and then only
request to probe for the specific port type suggested, e.g. if the
'hsotype' file tells us the port is "Control", remove QCDM probing; or,
if it tells us the port is "GPS", remove both AT and QCDM probings.

What do you think? Can we really always rely on the contents of the
'hsotype' files?

We could also use the same logic for Huawei and the GETPORTMODE result,
btw, once we fix the already known issues with the NDIS port reported.

-- 
Aleksander

>From 41d6e09185d2343344846173d51ff22823f3508b Mon Sep 17 00:00:00 2001
From: Aleksander Morgado <aleksander lanedo com>
Date: Wed, 31 Oct 2012 11:36:36 +0100
Subject: [PATCH] option,hso: early check of port type hints to avoid probings

Instead of getting the port type hints while grabbing each probed port, we now
run a custom init operation in the probing which already gives us the port type,
and therefore allows us to completely skip port probings.

Also now added hints for 'Diag' (QCDM) ports and AT/'Modem' ports (I guess it's
the one for PPP if we don't have a net port, which is unlikely anyway in HSO).

This makes the probing of an Option/HSO modem almost instant.
---
 plugins/option/mm-plugin-hso.c | 142 ++++++++++++++++++++++++++---------------
 1 file changed, 92 insertions(+), 50 deletions(-)

diff --git a/plugins/option/mm-plugin-hso.c b/plugins/option/mm-plugin-hso.c
index ce80a12..a11d608 100644
--- a/plugins/option/mm-plugin-hso.c
+++ b/plugins/option/mm-plugin-hso.c
@@ -32,6 +32,80 @@ int mm_plugin_major_version = MM_PLUGIN_MAJOR_VERSION;
 int mm_plugin_minor_version = MM_PLUGIN_MINOR_VERSION;
 
 /*****************************************************************************/
+/* Custom init */
+
+#define TAG_HSO_AT_CONTROL     "hso-at-control"
+#define TAG_HSO_AT_APP         "hso-at-app"
+#define TAG_HSO_AT_MODEM       "hso-at-modem"
+#define TAG_HSO_AT_GPS_CONTROL "hso-at-gps-control"
+#define TAG_HSO_GPS            "hso-gps"
+#define TAG_HSO_DIAG           "hso-diag"
+
+static gboolean
+hso_custom_init_finish (MMPortProbe *probe,
+                        GAsyncResult *result,
+                        GError **error)
+{
+    return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (result), error);
+}
+
+static void
+hso_custom_init (MMPortProbe *probe,
+                 MMAtSerialPort *port,
+                 GCancellable *cancellable,
+                 GAsyncReadyCallback callback,
+                 gpointer user_data)
+{
+    GUdevDevice *udev_port;
+    GSimpleAsyncResult *result;
+    const gchar *subsys, *sysfs_path;
+
+    subsys = mm_port_probe_get_port_subsys (probe);
+    udev_port = mm_port_probe_peek_port (probe);
+    sysfs_path = g_udev_device_get_sysfs_path (udev_port);
+
+    if (g_str_equal (subsys, "tty")) {
+        gchar *hsotype_path;
+        gchar *contents = NULL;
+
+        hsotype_path = g_build_filename (sysfs_path, "hsotype", NULL);
+        if (g_file_get_contents (hsotype_path, &contents, NULL, NULL)) {
+            if (g_str_has_prefix (contents, "Control")) {
+                g_object_set_data (G_OBJECT (probe), TAG_HSO_AT_CONTROL, GUINT_TO_POINTER (TRUE));
+                mm_port_probe_set_result_at (probe, TRUE);
+            } else if (g_str_has_prefix (contents, "Application")) {
+                g_object_set_data (G_OBJECT (probe), TAG_HSO_AT_APP, GUINT_TO_POINTER (TRUE));
+                mm_port_probe_set_result_at (probe, TRUE);
+            } else if (g_str_has_prefix (contents, "Modem")) {
+                g_object_set_data (G_OBJECT (probe), TAG_HSO_AT_MODEM, GUINT_TO_POINTER (TRUE));
+                mm_port_probe_set_result_at (probe, TRUE);
+            } else if (g_str_has_prefix (contents, "GPS Control")) {
+                g_object_set_data (G_OBJECT (probe), TAG_HSO_AT_GPS_CONTROL, GUINT_TO_POINTER (TRUE));
+                mm_port_probe_set_result_at (probe, TRUE);
+            } else if (g_str_has_prefix (contents, "GPS")) {
+                /* Not an AT port, but the port to grab GPS traces */
+                g_object_set_data (G_OBJECT (probe), TAG_HSO_GPS, GUINT_TO_POINTER (TRUE));
+                mm_port_probe_set_result_at (probe, FALSE);
+                mm_port_probe_set_result_qcdm (probe, FALSE);
+            } else if (g_str_has_prefix (contents, "Diag")) {
+                g_object_set_data (G_OBJECT (probe), TAG_HSO_DIAG, GUINT_TO_POINTER (TRUE));
+                mm_port_probe_set_result_qcdm (probe, TRUE);
+            }
+            g_free (contents);
+        }
+        g_free (hsotype_path);
+    }
+
+    result = g_simple_async_result_new (G_OBJECT (probe),
+                                        callback,
+                                        user_data,
+                                        hso_custom_init);
+    g_simple_async_result_set_op_res_gboolean (result, TRUE);
+    g_simple_async_result_complete_in_idle (result);
+    g_object_unref (result);
+}
+
+/*****************************************************************************/
 
 static MMBaseModem *
 create_modem (MMPlugin *self,
@@ -55,66 +129,29 @@ grab_port (MMPlugin *self,
            MMPortProbe *probe,
            GError **error)
 {
-    GUdevDevice *port;
-    const gchar *name, *subsys, *sysfs_path;
+    const gchar *name, *subsys;
     MMAtPortFlag pflags = MM_AT_PORT_FLAG_NONE;
-    gchar *devfile;
     MMPortType port_type;
 
-    port = mm_port_probe_peek_port (probe);
     subsys = mm_port_probe_get_port_subsys (probe);
     name = mm_port_probe_get_port_name (probe);
-
-    /* Build proper devfile path
-     * TODO: Why do we need to do this? If this is useful, a comment should be
-     * added explaining why; if it's not useful, let's get rid of it. */
-    devfile = g_strdup (g_udev_device_get_device_file (port));
-    if (!devfile) {
-        if (g_str_equal (subsys, "net")) {
-            /* Apparently 'hso' doesn't set up the right links for the netdevice,
-             * and thus libgudev can't get the sysfs file path for it.
-             */
-            devfile = g_strdup_printf ("/sys/class/net/%s", name);
-            if (!g_file_test (devfile, G_FILE_TEST_EXISTS)) {
-                g_free (devfile);
-                devfile = NULL;
-            }
-        }
-
-        if (!devfile) {
-            g_set_error (error,
-                         MM_CORE_ERROR,
-                         MM_CORE_ERROR_FAILED,
-                         "Could not get port's sysfs file.");
-            return FALSE;
-        }
-    }
-    g_free (devfile);
-
     port_type = mm_port_probe_get_port_type (probe);
-    sysfs_path = g_udev_device_get_sysfs_path (port);
 
     /* Detect AT port types */
     if (g_str_equal (subsys, "tty")) {
-        gchar *hsotype_path;
-        gchar *contents = NULL;
-
-        hsotype_path = g_build_filename (sysfs_path, "hsotype", NULL);
-        if (g_file_get_contents (hsotype_path, &contents, NULL, NULL)) {
-            if (g_str_has_prefix (contents, "Control"))
-                pflags = MM_AT_PORT_FLAG_PRIMARY;
-            else if (g_str_has_prefix (contents, "Application"))
-                pflags = MM_AT_PORT_FLAG_SECONDARY;
-            else if (g_str_has_prefix (contents, "GPS Control"))
-                pflags = MM_AT_PORT_FLAG_GPS_CONTROL;
-            else if (g_str_has_prefix (contents, "GPS")) {
-                /* Not an AT port, but the port to grab GPS traces */
-                g_assert (port_type == MM_PORT_TYPE_UNKNOWN);
-                port_type = MM_PORT_TYPE_GPS;
-            }
-            g_free (contents);
+        if (g_object_get_data (G_OBJECT (probe), TAG_HSO_AT_CONTROL))
+            pflags = MM_AT_PORT_FLAG_PRIMARY;
+        else if (g_object_get_data (G_OBJECT (probe), TAG_HSO_AT_APP))
+            pflags = MM_AT_PORT_FLAG_SECONDARY;
+        else if (g_object_get_data (G_OBJECT (probe), TAG_HSO_AT_GPS_CONTROL))
+            pflags = MM_AT_PORT_FLAG_GPS_CONTROL;
+        else if (g_object_get_data (G_OBJECT (probe), TAG_HSO_AT_MODEM))
+            pflags = MM_AT_PORT_FLAG_PPP;
+        else if (g_object_get_data (G_OBJECT (probe), TAG_HSO_GPS)) {
+            /* Not an AT port, but the port to grab GPS traces */
+            g_assert (port_type == MM_PORT_TYPE_UNKNOWN);
+            port_type = MM_PORT_TYPE_GPS;
         }
-        g_free (hsotype_path);
     }
 
     return mm_base_modem_grab_port (modem,
@@ -132,6 +169,10 @@ mm_plugin_create (void)
 {
     static const gchar *subsystems[] = { "tty", "net", NULL };
     static const gchar *drivers[] = { "hso", NULL };
+    static const MMAsyncMethod custom_init = {
+        .async  = G_CALLBACK (hso_custom_init),
+        .finish = G_CALLBACK (hso_custom_init_finish),
+    };
 
     return MM_PLUGIN (
         g_object_new (MM_TYPE_PLUGIN_HSO,
@@ -140,6 +181,7 @@ mm_plugin_create (void)
                       MM_PLUGIN_ALLOWED_DRIVERS,    drivers,
                       MM_PLUGIN_ALLOWED_AT,         TRUE,
                       MM_PLUGIN_ALLOWED_QCDM,       TRUE,
+                      MM_PLUGIN_CUSTOM_INIT,        &custom_init,
                       NULL));
 }
 
-- 
1.7.12.1