Hi Sebastian,

On 08/30/2017 11:30 AM, Sebastian Arnd wrote:
diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index 011713e..dcd8f14 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -67,6 +67,7 @@ struct gemalto_data {
        GAtChat *mdm;
        struct ofono_sim *sim;
        gboolean have_sim;
+       gboolean have_net;
        struct at_util_sim_state_query *sim_state_query;
        struct gemalto_hardware_monitor *hm;
  };
@@ -101,18 +102,31 @@ static void gemalto_debug(const char *str, void 
*user_data)
        ofono_info("%s%s", prefix, str);
  }
-static GAtChat *open_device(const char *device)
+static GAtChat *open_device(const char *device, char *device_debug_name)
  {
        GAtSyntax *syntax;
        GIOChannel *channel;
        GAtChat *chat;
+       GHashTable *options;
DBG("Opening device %s", device); - channel = g_at_tty_open(device, NULL);
+       options = g_hash_table_new(g_str_hash, g_str_equal);
+       if (options == NULL)
+               return NULL;
+       /*
+        * The modem does not answer when the "Baud" attibute is missing
+        * The supplied value is not making any change. But the "Baud"
+        * paramenter is needed.
+        */
+       g_hash_table_insert(options, "Baud", "115200");
+
+       channel = g_at_tty_open(device, options);
+       g_hash_table_destroy(options);
        if (channel == NULL)
                return NULL;
+

We do not allow double empty lines anywhere. Even if this were allowed, such whitespace changes are not related to the current commit and should be submitted separately.

        syntax = g_at_syntax_new_gsm_permissive();
        chat = g_at_chat_new(channel, syntax);
        g_at_syntax_unref(syntax);
@@ -121,6 +135,9 @@ static GAtChat *open_device(const char *device)
        if (chat == NULL)
                return NULL;
+ if (getenv("OFONO_AT_DEBUG"))
+               g_at_chat_set_debug(chat, gemalto_debug, device_debug_name);
+
        return chat;
  }
@@ -299,47 +316,54 @@ static int gemalto_hardware_monitor_enable(struct ofono_modem *modem)
        return 0;
  }
+

No double-empty lines please

  static int gemalto_enable(struct ofono_modem *modem)
  {
        struct gemalto_data *data = ofono_modem_get_data(modem);
-       const char *app, *mdm;
+       const char *app, *mdm, *net;
DBG("%p", modem); app = ofono_modem_get_string(modem, "Application");
        mdm = ofono_modem_get_string(modem, "Modem");
+       net = ofono_modem_get_string(modem, "NetworkInterface");
- if (app == NULL || mdm == NULL)
+       if (net != NULL)
+               data->have_net = TRUE;
+       else
+               data->have_net = FALSE;
+
+       if (app == NULL || (mdm == NULL && !data->have_net))
                return -EINVAL;
/* Open devices */
-       data->app = open_device(app);
+       data->app = open_device(app, "App");
        if (data->app == NULL)
                return -EINVAL;
- data->mdm = open_device(mdm);
-       if (data->mdm == NULL) {
-               g_at_chat_unref(data->app);
-               data->app = NULL;
-               return -EINVAL;
-       }
-
-       if (getenv("OFONO_AT_DEBUG")) {
-               g_at_chat_set_debug(data->app, gemalto_debug, "App");
-               g_at_chat_set_debug(data->mdm, gemalto_debug, "Mdm");
+       /* Mdm device is only requiered when there is no net interface */
+       if (!data->have_net) {
+               data->mdm = open_device(mdm, "Mdm");
+               if (data->mdm == NULL) {
+                       g_at_chat_unref(data->app);
+                       data->app = NULL;
+                       return -EINVAL;
+               }

With accordance to doc/coding-style.txt, item M1, please add an empty line here

+               g_at_chat_send(data->mdm, "ATE0", none_prefix,
+                                               NULL, NULL, NULL);
+               g_at_chat_send(data->mdm, "AT&C0", none_prefix,
+                                               NULL, NULL, NULL);
        }
- g_at_chat_send(data->mdm, "ATE0", none_prefix, NULL, NULL, NULL);
-       g_at_chat_send(data->app, "ATE0 +CMEE=1", none_prefix,
-                       NULL, NULL, NULL);
-       g_at_chat_send(data->mdm, "AT&C0", none_prefix, NULL, NULL, NULL);
+       g_at_chat_send(data->app, "ATE0", none_prefix, NULL, NULL, NULL);
+       g_at_chat_send(data->app, "AT+CMEE=1", none_prefix, NULL, NULL, NULL);
        g_at_chat_send(data->app, "AT&C0", none_prefix, NULL, NULL, NULL);
-
        g_at_chat_send(data->app, "AT+CFUN=4", none_prefix,
-                       cfun_enable, modem, NULL);
+                                               cfun_enable, modem, NULL);
gemalto_hardware_monitor_enable(modem); +

No double empty lines please

        return -EINPROGRESS;
  }
@@ -402,7 +426,8 @@ static void gemalto_set_online(struct ofono_modem *modem, ofono_bool_t online, DBG("modem %p %s", modem, online ? "online" : "offline"); - if (g_at_chat_send(data->app, command, NULL, set_online_cb, cbd, g_free))
+       if (g_at_chat_send(data->app, command, NULL,
+                                       set_online_cb, cbd, g_free))

Okay, but this should really be a separate commit to specifically address the '> 80 characters / line' issue.

                return;
CALLBACK_WITH_FAILURE(cb, cbd->data);
@@ -419,7 +444,8 @@ static void gemalto_pre_sim(struct ofono_modem *modem)
ofono_devinfo_create(modem, 0, "atmodem", data->app);
        ofono_location_reporting_create(modem, 0, "gemaltomodem", data->app);
-       sim = ofono_sim_create(modem, 0, "atmodem", data->app);
+       sim = ofono_sim_create(modem, OFONO_VENDOR_CINTERION,
+                                                       "atmodem", data->app);
if (sim && data->have_sim == TRUE)
                ofono_sim_inserted_notify(sim, TRUE);
@@ -438,7 +464,12 @@ static void gemalto_post_sim(struct ofono_modem *modem)
        ofono_sms_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
gprs = ofono_gprs_create(modem, 0, "atmodem", data->app);
-       gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
+
+       if (data->have_net)
+               gc = ofono_gprs_context_create(modem, 0,
+                                               "gemaltowwanmodem", data->app);

"gemaltomodem" would be better, unless you guys planning on multiple variants?

+       else
+               gc = ofono_gprs_context_create(modem, 0, "atmodem", data->mdm);
if (gprs && gc)
                ofono_gprs_add_context(gprs, gc);
@@ -450,7 +481,8 @@ static void gemalto_post_online(struct ofono_modem *modem)
DBG("%p", modem); - ofono_netreg_create(modem, OFONO_VENDOR_CINTERION, "atmodem", data->app);
+       ofono_netreg_create(modem, OFONO_VENDOR_CINTERION,
+                                                       "atmodem", data->app);

Same comment about '> 80 characters / line' as above. Separate these chunks into a separate commit and make it first in the series.

  }
static struct ofono_modem_driver gemalto_driver = {
diff --git a/plugins/udevng.c b/plugins/udevng.c
index aa28bcb..68c8386 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1080,13 +1080,24 @@ static gboolean setup_gemalto(struct modem_info* modem)
                                net = info->devnode;
                        else if (g_strcmp0(info->subsystem, "usbmisc") == 0)
                                qmi = info->devnode;
+               } else if (g_strcmp0(info->interface, "2/2/1") == 0) {
+                       if (g_strcmp0(info->number, "00") == 0)
+                               mdm = info->devnode;
+                       else if (g_strcmp0(info->number, "02") == 0)
+                               app = info->devnode;
+                       else if (g_strcmp0(info->number, "04") == 0)
+                               gps = info->devnode;
+               } else if (g_strcmp0(info->interface, "2/6/0") == 0 &&
+                         g_strcmp0(info->number, "0a") == 0) {

Please don't mix tabs & spaces. If this doesn't fit into an 80 character line, you can get creative, e.g.:

} else if (g_strcmp0(info->interface, "2/6/0") == 0) {
        if (g_strcmp0(info->number, "0a") == 0)
                net = ...;
}

+                               net = info->devnode;
                }
+

This whitespace change is spurious, please remove.

        }
DBG("application=%s gps=%s modem=%s network=%s qmi=%s",
                        app, gps, mdm, net, qmi);
- if (app == NULL || mdm == NULL)
+       if (app == NULL || (mdm == NULL && net == NULL))
                return FALSE;
ofono_modem_set_string(modem->modem, "Application", app);
@@ -1499,6 +1510,8 @@ static struct {
        { "gemalto",  "option",     "1e2d",       "0053"        },
        { "gemalto",  "cdc_wdm",    "1e2d",       "0053"        },
        { "gemalto",  "qmi_wwan",   "1e2d",       "0053"        },
+       { "gemalto",  "cdc_acm",    "1e2d",       "0061"        },
+       { "gemalto",  "cdc_ether",  "1e2d",       "0061"        },
        { "telit",    "cdc_ncm",    "1bc7", "0036"      },
        { "telit",    "cdc_acm",    "1bc7", "0036"      },
        { }


Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono

Reply via email to