[PATCH 2/2] dell: speed probing time up and reduce udev dependency

[Carlo Lobrano]

Hi Aleksander,

I still need to understand how to reply to old email while using git
send-mail (if possible).

However, attached you'll find the logs taken with this patch applied.

On 29 May 2017 at 09:48, Carlo Lobrano <c.lobrano at gmail.com> wrote:

> Currently Dell plugin implements a retry logic of three commands
> (AT+GMI, AT+CGMI, ATI1I2I3) to detect modem's vendor string.
> Moreover, since Telit modems always reply to the first command, to speed
> the probing time up, those modem are tagged with an Udev rule so that we
> can avoid sending the other two commands at all.
>
> However, the retry logic is in case a port needs some time to reply, so
> it makes sense to apply it only to the first command. Then if the port
> still
> does not respond with any kind of reply, that probably means that it is not
> AT capable and we can skip the other AT commands as well.
>
> Then, this patch:
> - sets a maximum number of timeouts for AT+GMI to 3. After this
>   timeouts, the port is considered not AT-capable.
> - sets AT+CGMI and ATI1I2I3 to be sent only once.
> - removes Dell udev rule for tagging Telit Modems.
>
> ---
>
> Patch v2:
>
> As per Aleksander's code review
>
> - mm_port_probe_set_result_at set to false when run out of retries for
> manufacturer (and yes, I do confirm that without this line, I saw other
> "AT" commands sent to the ports).
> - fixed debug logs
>
> @Aleksander I'm going to reply to this email with the logs you requested.
>
> ---
>  plugins/dell/77-mm-dell-port-types.rules |  4 ----
>  plugins/dell/mm-plugin-dell.c            | 29
> ++++++++++++++++++-----------
>  2 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/plugins/dell/77-mm-dell-port-types.rules
> b/plugins/dell/77-mm-dell-port-types.rules
> index bbb59b9..206fb6e 100644
> --- a/plugins/dell/77-mm-dell-port-types.rules
> +++ b/plugins/dell/77-mm-dell-port-types.rules
> @@ -8,9 +8,5 @@ GOTO="mm_dell_port_types_end"
>  LABEL="mm_dell_vendorcheck"
>  SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*",
> ENV{.MM_USBIFNUM}="$attr{bInterfaceNumber}"
>
> -# DW5580 is a Dell-branded Telit modem
> -# tag is needed here for dynamic port recognition
> -ATTRS{idVendor}=="413c", ATTRS{idProduct}=="81ba",
> ENV{ID_MM_TELIT_PORTS_TAGGED}="1"
> -
>  GOTO="mm_dell_port_types_end"
>  LABEL="mm_dell_port_types_end"
> diff --git a/plugins/dell/mm-plugin-dell.c b/plugins/dell/mm-plugin-dell.c
> index f9dfd1a..fbb0a9c 100644
> --- a/plugins/dell/mm-plugin-dell.c
> +++ b/plugins/dell/mm-plugin-dell.c
> @@ -45,6 +45,8 @@
>  #include "mm-broadband-modem-mbim.h"
>  #endif
>
> +#define MAX_PORT_PROBE_TIMEOUTS 3
> +
>  G_DEFINE_TYPE (MMPluginDell, mm_plugin_dell, MM_TYPE_PLUGIN)
>
>  MM_PLUGIN_DEFINE_MAJOR_VERSION
> @@ -70,6 +72,7 @@ typedef struct {
>      guint gmi_retries;
>      guint cgmi_retries;
>      guint ati_retries;
> +    guint timeouts;
>  } CustomInitContext;
>
>  static void
> @@ -140,6 +143,8 @@ static void custom_init_step (CustomInitContext *ctx);
>  static void
>  custom_init_step_next_command (CustomInitContext *ctx)
>  {
> +    ctx->timeouts = 0;
> +
>      if (ctx->gmi_retries > 0)
>          ctx->gmi_retries = 0;
>      else if (ctx->cgmi_retries > 0)
> @@ -169,6 +174,7 @@ response_ready (MMPortSerialAt *port,
>              return;
>          }
>          /* Directly retry same command on timeout */
> +        ctx->timeouts++;
>          custom_init_step (ctx);
>          g_error_free (error);
>          return;
> @@ -269,6 +275,13 @@ custom_init_step (CustomInitContext *ctx)
>      }
>  #endif
>
> +    if (ctx->timeouts >= MAX_PORT_PROBE_TIMEOUTS) {
> +        mm_dbg ("(Dell) couldn't detect real manufacturer in (%s): too
> many timeouts",
> +                mm_port_get_device (MM_PORT (ctx->port)));
> +        mm_port_probe_set_result_at (ctx->probe, FALSE);
> +        goto out;
> +    }
> +
>      if (ctx->gmi_retries > 0) {
>          ctx->gmi_retries--;
>          mm_port_serial_at_command (ctx->port,
> @@ -309,9 +322,10 @@ custom_init_step (CustomInitContext *ctx)
>          return;
>      }
>
> -    /* Finish custom_init */
> -    mm_dbg ("(Dell) couldn't flip secondary port to AT in (%s): all
> retries consumed",
> +    mm_dbg ("(Dell) couldn't detect real manufacturer in (%s): all
> retries consumed",
>              mm_port_get_device (MM_PORT (ctx->port)));
> +out:
> +    /* Finish custom_init */
>      g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>      custom_init_context_complete_and_free (ctx);
>  }
> @@ -337,15 +351,8 @@ dell_custom_init (MMPortProbe *probe,
>      ctx->port = g_object_ref (port);
>      ctx->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
>      ctx->gmi_retries = 3;
> -    ctx->cgmi_retries = 3;
> -    ctx->ati_retries = 3;
> -
> -    /* Dell-branded Telit modems always answer to +GMI
> -     * Avoid +CGMI and ATI sending for minimizing port probing time */
> -    if (mm_kernel_device_get_global_property_as_boolean (port_device,
> "ID_MM_TELIT_PORTS_TAGGED")) {
> -        ctx->cgmi_retries = 0;
> -        ctx->ati_retries = 0;
> -    }
> +    ctx->cgmi_retries = 1;
> +    ctx->ati_retries = 1;
>
>      custom_init_step (ctx);
>  }
> --
> 2.9.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170529/beaa38a5/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mm_dell_noudev_probing_speed_up.log
Type: text/x-log
Size: 147763 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170529/beaa38a5/attachment-0001.bin>