Awesome! Thank you!
Carlo On 29 May 2017 at 12:27, Aleksander Morgado <[email protected]> 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 >
_______________________________________________ ModemManager-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
