Hi Dan,
2015-03-23 17:46 GMT+01:00 Dan Williams <d...@redhat.com>:
> On Mon, 2015-03-23 at 17:16 +0100, Daniele Palmas wrote:
>> Adding dynamic port identification for Telit modems that support AT#PORTCFG
>> command. Port configurations for HE910/UE910/UL865 taken from document
>> "HE910/UE910/UL865 Families Ports Arrangements User Guide"
>> ---
>> plugins/telit/mm-plugin-telit.c | 252
>> +++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 249 insertions(+), 3 deletions(-)
>>
>> diff --git a/plugins/telit/mm-plugin-telit.c
>> b/plugins/telit/mm-plugin-telit.c
>> index 4783095..adfa4d7 100644
>> --- a/plugins/telit/mm-plugin-telit.c
>> +++ b/plugins/telit/mm-plugin-telit.c
>> @@ -21,6 +21,7 @@
>> #define _LIBMM_INSIDE_MM
>> #include <libmm-glib.h>
>>
>> +#include "mm-port-enums-types.h"
>> #include "mm-log.h"
>> #include "mm-modem-helpers.h"
>> #include "mm-plugin-telit.h"
>> @@ -33,6 +34,12 @@ int mm_plugin_minor_version = MM_PLUGIN_MINOR_VERSION;
>>
>>
>> /*****************************************************************************/
>>
>> +#define TAG_GETPORTCFG_SUPPORTED "getportcfg-supported"
>> +
>> +#define TAG_TELIT_MODEM_PORT "ID_MM_TELIT_PORT_TYPE_MODEM"
>> +#define TAG_TELIT_AUX_PORT "ID_MM_TELIT_PORT_TYPE_AUX"
>> +#define TAG_TELIT_NMEA_PORT "ID_MM_TELIT_PORT_TYPE_NMEA"
>> +
>> static MMBaseModem *
>> create_modem (MMPlugin *self,
>> const gchar *sysfs_path,
>> @@ -56,31 +63,54 @@ grab_port (MMPlugin *self,
>> GError **error)
>> {
>> GUdevDevice *port;
>> + MMDevice *device;
>> MMPortType ptype;
>> MMPortSerialAtFlag pflags = MM_PORT_SERIAL_AT_FLAG_NONE;
>>
>> port = mm_port_probe_peek_port (probe);
>> ptype = mm_port_probe_get_port_type (probe);
>> + device = mm_port_probe_peek_device (probe);
>>
>> /* Look for port type hints; just probing can't distinguish which port
>> should
>> * be the data/primary port on these devices. We have to tag them
>> based on
>> * what the Windows .INF files say the port layout should be.
>> + *
>> + * If no udev rules are found, AT#PORTCFG (if supported) can be used for
>> + * identifying the port layout
>> */
>> - if (g_udev_device_get_property_as_boolean (port,
>> "ID_MM_TELIT_PORT_TYPE_MODEM")) {
>> + if (g_udev_device_get_property_as_boolean (port, TAG_TELIT_MODEM_PORT))
>> {
>> mm_dbg ("telit: AT port '%s/%s' flagged as primary",
>> mm_port_probe_get_port_subsys (probe),
>> mm_port_probe_get_port_name (probe));
>> pflags = MM_PORT_SERIAL_AT_FLAG_PRIMARY;
>> - } else if (g_udev_device_get_property_as_boolean (port,
>> "ID_MM_TELIT_PORT_TYPE_AUX")) {
>> + } else if (g_udev_device_get_property_as_boolean (port,
>> TAG_TELIT_AUX_PORT)) {
>> mm_dbg ("telit: AT port '%s/%s' flagged as secondary",
>> mm_port_probe_get_port_subsys (probe),
>> mm_port_probe_get_port_name (probe));
>> pflags = MM_PORT_SERIAL_AT_FLAG_SECONDARY;
>> - } else if (g_udev_device_get_property_as_boolean (port,
>> "ID_MM_TELIT_PORT_TYPE_NMEA")) {
>> + } else if (g_udev_device_get_property_as_boolean (port,
>> TAG_TELIT_NMEA_PORT)) {
>> mm_dbg ("telit: port '%s/%s' flagged as NMEA",
>> mm_port_probe_get_port_subsys (probe),
>> mm_port_probe_get_port_name (probe));
>> ptype = MM_PORT_TYPE_GPS;
>> + } else if (g_object_get_data (G_OBJECT (device),
>> TAG_GETPORTCFG_SUPPORTED) != NULL) {
>> + if (g_strcmp0 (g_udev_device_get_property (port,
>> "ID_USB_INTERFACE_NUM"), g_object_get_data (G_OBJECT (device),
>> TAG_TELIT_MODEM_PORT)) == 0) {
>> + mm_dbg ("telit: AT port '%s/%s' flagged as primary",
>> + mm_port_probe_get_port_subsys (probe),
>> + mm_port_probe_get_port_name (probe));
>> + pflags = MM_PORT_SERIAL_AT_FLAG_PRIMARY;
>> + } else if (g_strcmp0 (g_udev_device_get_property (port,
>> "ID_USB_INTERFACE_NUM"), g_object_get_data (G_OBJECT (device),
>> TAG_TELIT_AUX_PORT)) == 0) {
>> + mm_dbg ("telit: AT port '%s/%s' flagged as secondary",
>> + mm_port_probe_get_port_subsys (probe),
>> + mm_port_probe_get_port_name (probe));
>> + pflags = MM_PORT_SERIAL_AT_FLAG_SECONDARY;
>> + } else if (g_strcmp0 (g_udev_device_get_property (port,
>> "ID_USB_INTERFACE_NUM"), g_object_get_data (G_OBJECT (device),
>> TAG_TELIT_NMEA_PORT)) == 0) {
>> + mm_dbg ("telit: port '%s/%s' flagged as NMEA",
>> + mm_port_probe_get_port_subsys (probe),
>> + mm_port_probe_get_port_name (probe));
>> + ptype = MM_PORT_TYPE_GPS;
>> + } else
>> + ptype = MM_PORT_TYPE_IGNORED;
>> } else {
>> /* If the port was tagged by the udev rules but isn't a primary or
>> secondary,
>> * then ignore it to guard against race conditions if a device just
>> happens
>> @@ -99,6 +129,216 @@ grab_port (MMPlugin *self,
>> }
>>
>>
>> /*****************************************************************************/
>> +/* Custom init */
>> +
>> +typedef struct {
>> + MMPortProbe *probe;
>> + MMPortSerialAt *port;
>> + GCancellable *cancellable;
>> + GSimpleAsyncResult *result;
>> + gboolean getportcfg_done;
>> + guint getportcfg_retries;
>> +} TelitCustomInitContext;
>> +
>> +static gboolean
>> +telit_custom_init_finish (MMPortProbe *probe,
>> + GAsyncResult *result,
>> + GError **error)
>> +{
>> + return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT
>> (result), error);
>> +}
>> +
>> +static void telit_custom_init_step (TelitCustomInitContext *ctx);
>> +
>> +static gboolean
>> +cache_port_mode (MMDevice *device,
>> + const gchar *reply)
>> +{
>> + GRegex *r = NULL;
>> + GRegexCompileFlags flags = G_REGEX_DOLLAR_ENDONLY | G_REGEX_RAW;
>> + GMatchInfo *match_info = NULL;
>> + GError *error = NULL;
>> + gboolean ret = FALSE;
>> + guint portcfg_current;
>> +
>> + /* #PORTCFG: <requested>,<active> */
>> + r = g_regex_new ("#PORTCFG:\\s*(\\d+),(\\d+)", flags, 0, NULL);
>> + g_assert (r != NULL);
>> +
>> + if (!g_regex_match_full (r, reply, strlen (reply), 0, 0, &match_info,
>> &error))
>> + goto out;
>> +
>> + if (!mm_get_uint_from_match_info (match_info, 2, &portcfg_current)) {
>> + mm_dbg ("telit: unrecognized #PORTCFG <active> value");
>> + goto out;
>> + }
>> +
>> + /* Reference for port configurations:
>> + * HE910/UE910/UL865 Families Ports Arrangements User Guide
>> + */
>> + switch (portcfg_current) {
>> + case 0:
>> + case 1:
>> + case 4:
>> + case 5:
>> + case 7:
>> + case 9:
>> + case 10:
>> + case 11:
>> + g_object_set_data_full (G_OBJECT (device), TAG_TELIT_MODEM_PORT,
>> g_strdup ("00"), (GDestroyNotify) g_free);
>> + g_object_set_data_full (G_OBJECT (device), TAG_TELIT_AUX_PORT,
>> g_strdup ("06"), (GDestroyNotify) g_free);
>
> You don't need the strdup() here, or the g_free, because you're just
> using a constant string. So these can all be just:
>
> g_object_set_data (G_OBJECT (device), TAG_..., "xx");
>
> and we save some memory.
>
Ok, I'll fix this in a V2 version of the patch.
>> + break;
>> + case 2:
>> + case 3:
>> + case 6:
>> + g_object_set_data_full (G_OBJECT (device), TAG_TELIT_MODEM_PORT,
>> g_strdup ("00"), (GDestroyNotify) g_free);
>> + break;
>> + case 8:
>> + case 12:
>> + g_object_set_data_full (G_OBJECT (device), TAG_TELIT_MODEM_PORT,
>> g_strdup ("00"), (GDestroyNotify) g_free);
>> + g_object_set_data_full (G_OBJECT (device), TAG_TELIT_AUX_PORT,
>> g_strdup ("06"), (GDestroyNotify) g_free);
>> + g_object_set_data_full (G_OBJECT (device), TAG_TELIT_NMEA_PORT,
>> g_strdup ("0a"), (GDestroyNotify) g_free);
>> + break;
>> + default:
>> + /* portcfg value not supported */
>> + goto out;
>> + }
>> + ret = TRUE;
>> +
>> +out:
>> + g_match_info_free (match_info);
>> + g_regex_unref (r);
>> + if (error != NULL) {
>> + mm_dbg ("telit: error while matching: %s", error->message);
>> + g_error_free (error);
>> + }
>> + return ret;
>> +}
>> +
>> +static void
>> +getportcfg_ready (MMPortSerialAt *port,
>> + GAsyncResult *res,
>> + TelitCustomInitContext *ctx)
>> +{
>> + const gchar *response;
>> + GError *error = NULL;
>> +
>> + response = mm_port_serial_at_command_finish (port, res, &error);
>> + if (error) {
>> + mm_dbg ("telit: couldn't get port mode: '%s'",
>> + error->message);
>> +
>> + /* If ERROR or COMMAND NOT SUPPORT occur then do not retry the
>> + * command.
>> + */
>> + if (g_error_matches (error,
>> + MM_MOBILE_EQUIPMENT_ERROR,
>> + MM_MOBILE_EQUIPMENT_ERROR_UNKNOWN))
>> + ctx->getportcfg_done = TRUE;
>> + } else {
>> + MMDevice *device;
>> + device = mm_port_probe_peek_device (ctx->probe);
>> +
>> + /* Results are cached in the parent device object */
>> + if (g_object_get_data (G_OBJECT (device), TAG_GETPORTCFG_SUPPORTED)
>> == NULL) {
>> + mm_dbg ("telit: retrieving port mode layout");
>> + if (cache_port_mode (device, response)) {
>> + g_object_set_data (G_OBJECT (device),
>> TAG_GETPORTCFG_SUPPORTED, GUINT_TO_POINTER (TRUE));
>> + ctx->getportcfg_done = TRUE;
>> + }
>> + }
>> +
>> + /* Port answered to #PORTCFG, so mark it as being AT already */
>> + mm_port_probe_set_result_at (ctx->probe, TRUE);
>> + }
>> +
>> + if (error)
>> + g_error_free (error);
>> +
>> + telit_custom_init_step (ctx);
>> +}
>> +
>> +static void
>> +telit_custom_init_context_complete_and_free (TelitCustomInitContext *ctx)
>> +{
>> + g_simple_async_result_complete_in_idle (ctx->result);
>> +
>> + if (ctx->cancellable)
>> + g_object_unref (ctx->cancellable);
>> + g_object_unref (ctx->port);
>> + g_object_unref (ctx->probe);
>> + g_object_unref (ctx->result);
>> + g_slice_free (TelitCustomInitContext, ctx);
>> +}
>> +
>> +static void
>> +telit_custom_init_step (TelitCustomInitContext *ctx)
>> +{
>> + GUdevDevice *port;
>> +
>> + /* If cancelled, end */
>> + if (g_cancellable_is_cancelled (ctx->cancellable)) {
>> + mm_dbg ("telit: no need to keep on running custom init in (%s)",
>> + mm_port_get_device (MM_PORT (ctx->port)));
>> + goto out;
>> + }
>> +
>> + /* Try to get a port configuration from the modem: usb interface 00
>> + * is always linked to an AT port
>> + */
>> + port = mm_port_probe_peek_port (ctx->probe);
>> + if (!ctx->getportcfg_done &&
>> + !g_udev_device_get_property_as_boolean (port,
>> "ID_MM_TELIT_DISABLE_GETPORTCFG") &&
>
> Hmm, is there a real need to disable the PORTCFG testing, or just done
> to speed up probing? Maybe instead just skip the custom probing if the
> modem has no existing udev tags, which could be done by adding another
> udev tag like ID_MM_TELIT_PORTS_TAGGED for any modems in the rules file
> that have explicit port tags.
>
> Dan
>
Not a real need, but just to speed up. I will try to modify things for
using the new udev tag.
Thanks for the review.
Regards,
Daniele
>> + g_strcmp0 (g_udev_device_get_property (port,
>> "ID_USB_INTERFACE_NUM"), "00") == 0) {
>> +
>> + if (ctx->getportcfg_retries == 0)
>> + goto out;
>> + ctx->getportcfg_retries--;
>> +
>> + mm_port_serial_at_command (
>> + ctx->port,
>> + "AT#PORTCFG?",
>> + 2,
>> + FALSE, /* raw */
>> + FALSE, /* allow_cached */
>> + ctx->cancellable,
>> + (GAsyncReadyCallback)getportcfg_ready,
>> + ctx);
>> + return;
>> + }
>> +
>> +out:
>> + g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>> + telit_custom_init_context_complete_and_free (ctx);
>> +}
>> +
>> +static void
>> +telit_custom_init (MMPortProbe *probe,
>> + MMPortSerialAt *port,
>> + GCancellable *cancellable,
>> + GAsyncReadyCallback callback,
>> + gpointer user_data)
>> +{
>> + MMDevice *device;
>> + TelitCustomInitContext *ctx;
>> +
>> + device = mm_port_probe_peek_device (probe);
>> +
>> + ctx = g_slice_new (TelitCustomInitContext);
>> + ctx->result = g_simple_async_result_new (G_OBJECT (probe),
>> + callback,
>> + user_data,
>> + telit_custom_init);
>> + ctx->probe = g_object_ref (probe);
>> + ctx->port = g_object_ref (port);
>> + ctx->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
>> + ctx->getportcfg_done = FALSE;
>> + ctx->getportcfg_retries = 3;
>> +
>> + telit_custom_init_step (ctx);
>> +}
>> +
>> +/*****************************************************************************/
>>
>> G_MODULE_EXPORT MMPlugin *
>> mm_plugin_create (void)
>> @@ -111,6 +351,11 @@ mm_plugin_create (void)
>> "ID_MM_TELIT_TAGGED",
>> NULL
>> };
>> + /* Custom init for port identification */
>> + static const MMAsyncMethod custom_init = {
>> + .async = G_CALLBACK (telit_custom_init),
>> + .finish = G_CALLBACK (telit_custom_init_finish),
>> + };
>>
>> return MM_PLUGIN (
>> g_object_new (MM_TYPE_PLUGIN_TELIT,
>> @@ -119,6 +364,7 @@ mm_plugin_create (void)
>> MM_PLUGIN_ALLOWED_VENDOR_IDS, vendor_ids,
>> MM_PLUGIN_ALLOWED_AT, TRUE,
>> MM_PLUGIN_ALLOWED_UDEV_TAGS, udev_tags,
>> + MM_PLUGIN_CUSTOM_INIT, &custom_init,
>> NULL));
>> }
>>
>
>
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
