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

[Aleksander Morgado]

On 29/05/17 09:48, Carlo Lobrano 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.
> 

Pushed to git master, thanks!

The logs look good to be, btw, thanks for sending them.

> ---
>  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);
>  }
> 


-- 
Aleksander
https://aleksander.es