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

[Carlo Lobrano]

Awesome! Thank you!


Carlo

On 29 May 2017 at 12:27, Aleksander Morgado <aleksander at aleksander.es>
wrote:

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170529/485f9c97/attachment.html>