[PATCH 2/2] dell: removed ID_MM_TELIT_PORTS_TAGGED dependency

[Aleksander Morgado]

Hey,

On Fri, May 5, 2017 at 4:49 PM, Carlo Lobrano <c.lobrano at gmail.com> wrote:
> Currently Dell plugin depends on ID_MM_TELIT_PORTS_TAGGED
> environment variable, set by udev, to speed up probing time
> avoiding +CGMI and ATI sending.
>
> To remove this dependency from udev, I moved the product ID
> check from dell's udev rule to mm-plugin-dell.c file.

I don't see any clear benefit in doing this; we're moving the match
with the PID from the udev rules file to the actual source code, which
means that if we ever get any other Dell-branded modem we'll need to
update the source code instead of adding a new udev rules file. But...

Looking at the actual purpose of the code; it looks like it's for
minimizing probing time because Telit modems always reply well to
AT+GMI , and that is actually the first command we try... so do we
even need that logic there for anything? What happens if you remove
the block setting the cgmi_retries and at_retries to 0?

> ---
>  plugins/dell/77-mm-dell-port-types.rules | 2 +-
>  plugins/dell/mm-plugin-dell.c            | 8 +++++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/plugins/dell/77-mm-dell-port-types.rules b/plugins/dell/77-mm-dell-port-types.rules
> index bbb59b9..5dcef47 100644
> --- a/plugins/dell/77-mm-dell-port-types.rules
> +++ b/plugins/dell/77-mm-dell-port-types.rules
> @@ -10,7 +10,7 @@ SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*", ENV{.MM_USBIFNUM}="$attr{bInte
>
>  # 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"
> +#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..71ca5a4 100644
> --- a/plugins/dell/mm-plugin-dell.c
> +++ b/plugins/dell/mm-plugin-dell.c
> @@ -325,6 +325,7 @@ dell_custom_init (MMPortProbe *probe,
>  {
>      CustomInitContext *ctx;
>      MMKernelDevice *port_device;
> +    guint16 product_id;
>
>      port_device = mm_port_probe_peek_port (probe);
>
> @@ -342,9 +343,14 @@ dell_custom_init (MMPortProbe *probe,
>
>      /* 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")) {
> +    product_id = mm_device_get_product (mm_port_probe_peek_device (probe));
> +    switch (product_id) {
> +    case 0x81ba:
>          ctx->cgmi_retries = 0;
>          ctx->ati_retries = 0;
> +        break;
> +    default:
> +        break;
>      }
>
>      custom_init_step (ctx);
> --
> 2.9.3
>
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel



-- 
Aleksander
https://aleksander.es